-
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
feat(react-native): enable hermes engine #15279
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.
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.
✅ This pull request was succesfully scanned and it was determined that it does not contain any changes that present immediate security concerns. If you would still like for it to be reviewed by an expert from our reviewer community, you can submit it manually via the HackerOne PullRequest dashboard
e4de48b
to
a532a16
Compare
37b5cef
to
4ef9736
Compare
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.
Great stuff! Left some comments, PTAL. Please rebase with master too, so the new SDK CI tests run.
You can also have some stuff to delete:
- The part which pushes the JSC library to maven, in the android release-sdk.sh script
android/sdk/src/main/java/org/jitsi/meet/sdk/ReactInstanceManagerHolder.java
Outdated
Show resolved
Hide resolved
android/sdk/src/main/java/org/jitsi/meet/sdk/ReactInstanceManagerHolder.java
Outdated
Show resolved
Hide resolved
android/sdk/src/main/java/org/jitsi/meet/sdk/ReactInstanceManagerHolder.java
Outdated
Show resolved
Hide resolved
4ef9736
to
9e5a54e
Compare
25ff543
to
eafa210
Compare
199d4ab
to
6805d62
Compare
hermes and reat-native versions will be the same, so maybe no need to have a different hermesVersion variable |
Oh, is that so? Are they always released in tandem? |
Certainly looks that way: https://mvnrepository.com/artifact/com.facebook.react/hermes-android @Calinteodor can you remove the extra variable pl? @keremoge thanks for the heads up! |
No description provided.