-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
dep(react-native): version 0.72.5 update #13897
Conversation
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.
Some initial comments! Thanks for taking care of this!
3dd14e8
to
91b82d7
Compare
91b82d7
to
6e39e45
Compare
android/build.gradle
Outdated
@@ -122,7 +129,7 @@ allprojects { | |||
project.version = "${json.version}-jitsi-${versionQualifierNumber}" | |||
|
|||
task jitsiAndroidSourcesJar(type: Jar) { | |||
classifier = 'sources' | |||
archiveClassifier = 'sources' |
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.
classifier deprecated starting gradle 8.0
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.
Let's drop the Gradle update, it's a major one and should be handled separately.
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.
Some quick observations.
android/app/build.gradle
Outdated
@@ -75,6 +76,9 @@ android { | |||
} | |||
|
|||
dependencies { | |||
|
|||
// The version of react-native is set by the React Native Gradle Plugin | |||
implementation('com.facebook.react:react-android') |
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.
Same as above, let's drop this for now, and reconsider later, but even if we end up using it, it would be in the SDK gradle, not the app.
android/app/build.gradle
Outdated
@@ -173,5 +177,5 @@ gradle.projectsEvaluated { | |||
} | |||
|
|||
if (googleServicesEnabled) { | |||
apply plugin: 'com.google.gms.google-services' | |||
apply plugin: "com.google.gms.google-services" |
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.
Pl undo.
android/build.gradle
Outdated
@@ -122,7 +129,7 @@ allprojects { | |||
project.version = "${json.version}-jitsi-${versionQualifierNumber}" | |||
|
|||
task jitsiAndroidSourcesJar(type: Jar) { | |||
classifier = 'sources' | |||
archiveClassifier = 'sources' |
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.
Let's drop the Gradle update, it's a major one and should be handled separately.
ios/Podfile
Outdated
|
||
install! 'cocoapods', :deterministic_uuids => false | ||
|
||
production = ENV["PRODUCTION"] == "1" |
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.
What happened to this?
package.json
Outdated
@@ -65,7 +66,7 @@ | |||
"js-md5": "0.6.1", | |||
"js-sha512": "0.8.0", | |||
"jwt-decode": "2.2.0", | |||
"lib-jitsi-meet": "https://github.com/jitsi/lib-jitsi-meet/releases/download/v1716.0.0+93c167d3/lib-jitsi-meet.tgz", | |||
"lib-jitsi-meet": "https://github.com/jitsi/lib-jitsi-meet/releases/download/v1713.0.0+a1d7b0ea/lib-jitsi-meet.tgz", |
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.
Drop this change, ljm should not be downgraded.
"@babel/plugin-proposal-export-default-from": "7.16.0", | ||
"@babel/preset-env": "7.16.0", | ||
"@babel/preset-react": "7.16.0", | ||
"@babel/core": "7.22.5", |
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.
IS this change absolutely necessary? If not I'd say we drop it to narrow down the scope.
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.
I can test if it works without this update. It is recommended.
patches/react-native+0.72.6.patch
Outdated
- if (!self->_inBackground && [RCTSharedApplication() applicationState] == UIApplicationStateBackground) { | ||
- [self appDidMoveToBackground]; | ||
- } | ||
+ | ||
+ __block BOOL initialInBackground; | ||
+ dispatch_sync(dispatch_get_main_queue(), ^{ |
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.
We need to update the patch here. They are alrey running the check on the main queue above, which is great. It just needs to include the check for the proximity state.
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.
I submitted out patch upstream: facebook/react-native#41262 Perhaps you can just take that.
333b53c
to
d030824
Compare
d030824
to
f8a853e
Compare
PR continues here #14130 |
No description provided.