-
Notifications
You must be signed in to change notification settings - Fork 984
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
PR to upgrade react-native
to 0.72.5 and many other things
#17241
Conversation
Jenkins BuildsClick to see older builds (421)
|
a0c4917
to
d15fbaf
Compare
For Android we see this issue consistently in dev/debug environment |
✔️ status-mobile/prs/ios/PR-17241#6 🔹 ~9 min 1 sec 🔹 15ae7e9 🔹 📦 ios package |
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.
Looking good, the only pain point for me is the bunch of exceptions added in nix/deps/gradle/generate.sh
. But I don't want to block on just that, and we can find a better solution later, especially if someone gets to work on:
@siddarthkay we have not started proper testing of this PR yet as we are getting ready for testing release. But I have installed latest PR builds and already faced couple of issues. Please, take a look. ISSUE 1 App crashes on enabling biometry during onboardingSteps:
Actual result: Video and logs are taken from IOS. Screenshot with error is taken from Android telegram-cloud-document-2-5474548135059535025.mp4 |
ISSUE 2 Some of Settings screens are not loaded (Android)Reproducing on Android 12, Samsung Galaxy A52. Steps:
Actual result: screens are empty. Error is displayed telegram-cloud-document-2-5474548135059535067.mp4Aside from this for some reason Android security and log level settings are set to blocking screenshots and log level set to "info". That's why I cannot make any screenshots and send any logs from Android. Unable to change those settings due to current issue. |
ISSUE 3 App crashes during logoutSteps:
Actual result: app crashes Video and logs are taken from IOS. Error photo with is taken from Android telegram-cloud-document-2-5474548135059535077.mp4 |
@siddarthkay sorry, I have missed this conversation https://discord.com/channels/1103692771585433630/1148918637638471740 Now I see that some of above issues are known. |
Hi @pavloburykh : no worries, happy to work on these early issues :) 👍🏻 |
7b23fef
to
9483f82
Compare
9483f82
to
83b6d5b
Compare
Have to figure these out first. |
3798d35
to
1b2bed5
Compare
c4327fd
to
44846c1
Compare
@siddarthkay could you please check why IOS build is failing https://ci.status.im/job/status-mobile/job/prs/job/ios/job/PR-17241/129/consoleText ? I have tried to rebuild but it didn't help. |
Hi @pavloburykh : I'll remove last 2 commits and build will work, I'll let you know once it's done. |
@siddarthkay also quoting this one just in case you have missed. Thank you! |
3aead95
to
c2b64b6
Compare
@siddarthkay integration tests are failed, would you mind to check? |
c2b64b6
to
6675b31
Compare
|
f5988bf
to
757bec4
Compare
looks like praying worked! |
76% of end-end tests have passed
Failed tests (5)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (7)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Passed tests (37)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
|
@siddarthkay, thank you for your excellent work on this important PR! It is a great honor for me to announce that we are ready to merge it!!! |
d958123
to
af5377f
Compare
Thanks @pavloburykh and everyone at @status-im/mobile-qa for testing this PR. note: I've pointed status-go back to develop version for merging. |
af5377f
to
008ee0d
Compare
This commit does many things : - Upgrade `react-native ` to `0.72.5` - Upgrade `react-native-reanimated` to `3.5.4` - Upgrade `react-native-navigation` to `7.37.0` - `ndkVersion` has been bumped to `25.2.9519653` - `cmakeVersion` has been bumped to `3.22.1` - `kotlinVersion` has been bumped to `1.7.22` - `AGP` has been bumped to `7.4.2` - `Gradle` has been upgraded to `8.0.1` - Android `CompileSDK` and `TargetSDK` have been bumped to 33 - `@react-native-async-storage/async-storage` has been upgraded to `1.19.3` - `@walletconnect/client` has been nuked - some of the old `react-native-reanimated` code has been nuked - `react-native-keychain` fork has been replaced with `8.1.2` - On Android we are currently relying on `Hermes` Engine. - On iOS we are currently relying on JSC - We are not enabling new architecture for now (I have plans for that in the future) ref: #18138 IOS only PR : #16721 Android only PR : #17062 - `make run-metro` now has a target of `android` which was `clojure` earlier, this will increase the time it takes to start metro terminal but this is needed otherwise you will get a nasty error while developing for android locally.
008ee0d
to
6924070
Compare
The metro terminal no longer needs to have `android` target anymore. I had to do this in #17241 This commit sets the target of metro terminal back to `clojure`. I tested building `android` and `iOS` on my MacOS and on my linux machines and found no side effect. Now metro terminal is fast again.
Summary
This PR does many things :
react-native
to0.72.5
react-native-reanimated
to3.5.4
react-native-navigation
to7.37.0
ndkVersion
has been bumped to25.2.9519653
cmakeVersion
has been bumped to3.22.1
kotlinVersion
has been bumped to1.7.22
AGP
has been bumped to7.4.2
Gradle
has been upgraded to8.0.1
CompileSDK
andTargetSDK
have been bumped to 33@react-native-async-storage/async-storage
has been upgraded to1.19.3
@walletconnect/client
has been nukedreact-native-reanimated
code has been nukedreact-native-keychain
fork has been replaced with8.1.2
Notes
Hermes
Engine.IOS only PR : #16721
Android only PR : #17062
Known Side Effects
make run-metro
now has a target ofandroid
which wasclojure
earlier, this will increase the time it takes to start metro terminal but this is needed otherwise you will get a nasty error while developing for android locally.To Do