Skip to content
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

Merged
merged 4 commits into from
Jul 29, 2021

Conversation

mikehardy
Copy link
Collaborator

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 then yarn example:ios yarn example:android - and for me anyway, they work.

The previous example appeared broken

Supercedes / Closes #68
Related #21

Interested party @KrisLau

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
@mikehardy mikehardy changed the title New example auto-generation script + results / docs changes to use it docs(example): new example auto-generation script + results / docs changes to use it Jul 6, 2021
Copy link
Owner

@thebergamo thebergamo left a 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
Copy link
Owner

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?

Copy link
Collaborator Author

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

Copy link
Collaborator Author

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

README.md Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 7, 2021

🎉 This PR is included in version 4.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants