fix: remove redundant "enabled" property #449
Conversation
README.md
Outdated
| #### Android | ||
| - Set `newArchEnabled` to true inside `example/android/gradle.properties` | ||
| - Run `npm run example-android` | ||
| - Run `npm run example-android` _(first run might fail, try to run it again)_ |
There was a problem hiding this comment.
This is worrying... What do we mean here by "first run"? Do we mean the build? Or the app launch?
We don't have anything like that reported and I didn't observe it yet neither in my work with example app, neither in CI builds.
Let me double check it so we make 100% sure it's not a local issue of yours that we put to official docs - for that can you create also the issue in repo so I can check it in repro app?
What I'm trying to say: if you observe such issue then for sure it's totally something to be checked and handled. But let's do that before we put such statement in docs.
There was a problem hiding this comment.
This happened in this scenario:
- clone project
- run install
- run android on the old arch
- change settings to the new arch
- run android on the new arch (without cleaning android <-- might be the problem)
It might have failed on some of the downloads that were going on, but not sure. If you prefer, I can remove the note in the README. Just put it there so people might try it out once more before creating an issue
There was a problem hiding this comment.
@slavikdenis Thanks for explaining 👍
Hm, so if this issue appears without cleaning, then I'd prefer to have a note in README that the clean of Android build is suggested when building new arch on an old arch.
Yes, let's remove that from this PR - I think that no matter what kind of note we decide to put in README it's always better to separate such changes between two PRs.
But thank you for noticing this and letting me know, it's definitely something worth revisiting.
There was a problem hiding this comment.
Removed via 8303f8a
Let's see if it occurs to anyone else 👍
| In order to use the new architecture some extra steps are needed. | ||
| #### iOS | ||
| - Install pods with new arch flag inside `example/ios` folder: `RCT_NEW_ARCH_ENABLED=1 pod install` | ||
| - Run `npm run example-ios` |
There was a problem hiding this comment.
Let's remove that change.
Having this like so is intentional - it guides how to launch the example from project's root where the README is.
We can add the proposed step as additional, alternative like: "or run npm run ios(...)" but still it should be done in separate PR. This PR is about redundant enabled property.
2 participants
Summary:
Removing redundant "enabled" property as discussed in #424 (comment) and documented in #426
Fixed some instructions in README as well:
npm run iosinexample/iosfolder" instead of "npm run example-ios" <-- the other command runspod-installwhich installs pods with old archTest Plan:
Enabled "disabled" example for all platforms, so run the examples and test it out :)
iOS:
Android:
