Skip to content

Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties#35108

Closed
cortinico wants to merge 1 commit into
facebook:mainfrom
cortinico:export-D40762872
Closed

Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties#35108
cortinico wants to merge 1 commit into
facebook:mainfrom
cortinico:export-D40762872

Conversation

@cortinico
Copy link
Copy Markdown
Contributor

@cortinico cortinico commented Oct 27, 2022

Summary:
I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the gradle.properties file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Differential Revision: D40762872

@cortinico cortinico requested a review from hramos as a code owner October 27, 2022 15:53
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Oct 27, 2022
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Oct 27, 2022

This pull request was exported from Phabricator. Differential Revision: D40762872

@react-native-bot react-native-bot added the Platform: Android Android applications. label Oct 27, 2022
ecreeth
ecreeth reviewed Oct 27, 2022
View reviewed changes
Comment thread template/android/gradle.properties Outdated
Copy link
Copy Markdown
Contributor

@ecreeth ecreeth Oct 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Oct 27, 2022

This pull request was exported from Phabricator. Differential Revision: D40762872

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Oct 27, 2022

This pull request was exported from Phabricator. Differential Revision: D40762872

1 similar comment
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Oct 27, 2022

This pull request was exported from Phabricator. Differential Revision: D40762872

@cortinico @facebook-github-bot
Cleanup the template documentation after RNGP & hermesEnabled to grad…
be72feb 
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: 58fd2c87b3460da21f2560d7beb93710b626cda5
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Oct 27, 2022

This pull request was exported from Phabricator. Differential Revision: D40762872

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Oct 27, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,001,888 +495
android hermes armeabi-v7a 6,378,328 +461
android hermes x86 7,414,780 +832
android hermes x86_64 7,278,674 +802
android jsc arm64-v8a 8,867,565 +0
android jsc armeabi-v7a 7,605,882 +0
android jsc x86 8,925,465 +0
android jsc x86_64 9,408,701 +0

Base commit: ba5454c
Branch: main

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Oct 27, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ba5454c
Branch: main

@pull-bot
Copy link
Copy Markdown

pull-bot commented Oct 27, 2022

PR build artifact for be72feb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link
Copy Markdown

pull-bot commented Oct 27, 2022

PR build artifact for be72feb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@react-native-bot
Copy link
Copy Markdown
Collaborator

react-native-bot commented Oct 27, 2022

This pull request was successfully merged by @cortinico in 6a43faf.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hramos hramos Awaiting requested review from hramos

1 more reviewer

@ecreeth ecreeth ecreeth left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cortinico @facebook-github-bot @analysis-bot @pull-bot @react-native-bot @ecreeth