Fix/tracing strategy bt #1252
Fix/tracing strategy bt #1252
Conversation
There was a problem hiding this comment.
Works like a charm 👏👏👏👏 🥳🥳🥳🥳
This PR also addresses the issue we have open: lugg/react-native-config#467
| @@ -161,6 +161,10 @@ android { | |||
|
|
|||
| versionCode 11 | |||
| versionName "1.0.8" | |||
|
|
|||
| //Fix problem in bt where .env.bt is not recognized | |||
| resValue "string", "build_config_package", "org.pathcheck.covidsafepaths" | |||
There was a problem hiding this comment.
Oh wow, this is great!!! just tested it and it definitely fixes the problem. If you have the time would you mind educating me on what the issue was(totally up to you 😄 )? Also, we had an open issue on the react-native-config package lugg/react-native-config#467 and this fixes that question we opened if you are ok with linking this PR to the issue we can close it right away. I'll wait a bit for some other folks to check it out but I'm more than happy with merging this. 🎉🎉🎉🎉🎉
There was a problem hiding this comment.
I set a breakpoint in the react-native-config file ReactNativeConfigModule.java. I saw that it was trying to get the Config constants from org.pathcheck.covidsafepathsBt.BuildConfig, which didn't exist. So I tried changing the path to org.pathcheck.covidsafepaths.BuildConfig, and that worked. Then I remembered this from the react-native-config README:
In `android/app/build.gradle`, if you use `applicationIdSuffix` or `applicationId` that is different from the package name indicated in `AndroidManifest.xml` in `<manifest package="...">` tag, for example, to support different build variants:
Add this in `android/app/build.gradle`
```
defaultConfig {
...
resValue "string", "build_config_package", "YOUR_PACKAGE_NAME_IN_ANDROIDMANIFEST.XML"
}
I didn't see that the condition that was stated applied, but I decided to try it anyway, and I was lucky
|
No problem |
|
@crayne Can you please rebase/merge latest from develop? I think that might fix the build issue. |
|
@crayne Actually, I believe it's because your are submitting a PR from a fork which lacks access to |
|
No change after I merge the latest from develop. |
Thanks for trying that, nothing else to do on your end, seems that the failure is related to a configuration on the CI. We're trying to get it fixed so we can add this commit in there. |
|
We don't have a fix at this time, but @aledustet ran this build and it passed CI: So when the time comes we will be able to merge it. @JacobJaffe Can you give final sign off on this? I want to make sure it doesn't introduce any regressions for you on the GPS side. |
|
Did the test work because it was built from the main repo, not from a fork? If this is true does it mean anything else I do on a fork will have the same problem? |
For some context here, the submodule for the mobile-resources that was added about a week ago broke ci for prs from forks. The quick fix here is to either turn off the required flag on those tests, or open a new pr with the commit from the main repo. @mattThousand , what are your thoughts on this one? should we remove the submodules and take another pass at this in a future date so we unblock external contributions? |
Currently, the CONTRIBUTING.md document asks developers to fork the repo and develop off of that, so it would be nice if people who follow those instructions can contribute functional PRs. |
|
I agree with Jay. Otherwise, any changes made to a fork will need to be re-created on the main repo. The other alternative would be to do away with the fork requirement. |
Description:
Fixes crash on launch of bt build variant because .env.bt is not read correctly
Linked issues:
Fixes #1221
Fixes lugg/react-native-config#467
Screenshots:
How to test:
yarn run-android-bt -- no crash in App.js