-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(example): new example auto-generation script + results / docs changes to use it #69
Conversation
This should get us off the treadmill of updating examples, forever
…mple.sh The contents here will change periodically. Only App.js and README.md are preserved when re-generated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks great! Thanks for this improved example :)
@@ -74,8 +74,8 @@ jobs: | |||
restore-keys: | | |||
${{ runner.os }}-yarn- | |||
- name: Install dependencies | |||
run: yarn install | |||
run: yarn install && cd RNFBSDKExample && yarn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to run yarn twice here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of them is at the top level, one of them is in the example. The example constructed here is less "magical" in that it does not automatically / auto-magically reference the parent directory code. That's a little inconvenient if using the example app for module development - and I could definitely see an improvement in the future that switches the new-style example to again reference the code raw on the filesystem from the parent directory, but in this style the example is a fully-standalone / not-integrated-with-anything react-native app. So that means you need to go into it and yarn install before it will run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(note, to implement this change - to have the example again reference the parent directory module code, I think it's just a switch in build.gradle/Podfile/metro.config.js to put the references in place, or alternatively perhaps it can just be an override of the yarn add
command to be yarn add ../
or something? But then it will still be referencing just the module, not the rest of the modules and thus it will still need a yarn run in the example itself
🎉 This PR is included in version 4.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
There is nothing more useless for module maintainers to do than try to keep up with react-native updates in their example apps.
But we're programmers, so we can just auto-generate an example via a script, then just re-run the script to update it.
I do this in a lot of other places to good effect:
And now I propose it here.
Each commit is separate so may be reviewed with a relevant commit message, since the actual new example commit is huge (but also purely boilerplate / ignorable) as is the commit that kills the old example. Trying to look at the whole diff is a horrible thought, please do not torture yourself :-)
The test plan for this is to just keep re-running
refresh-example.sh
thenyarn example:ios
yarn example:android
- and for me anyway, they work.The previous example appeared broken
Supercedes / Closes #68
Related #21
Interested party @KrisLau