Update android build.gradle template setup #135

friederbluemle · 2019-09-18T17:32:00Z

This wraps the buildscripts section in the generated build.gradle file in a conditional, avoiding unnecessary downloads of/conflicts with different Android Gradle plugin distributions in application projects.

Please check facebook/react-native#25569 for a related discussion, specifically this comment: facebook/react-native#25569 (comment)

The commit also includes some minor indentation fixes in the same file.

Closes #129

_{^{Once approved, please use a normal GitHub merge (i.e. NO rebase/squash merge) to integrate the commit(s) from the PR head branch. The changes are broken up into meaningful, atomic commits, and my branch should already be up-to-date with the latest base branch. If it isn't, or if you want me to change anything, please let me know, and I will update the branch as soon as possible. Thank you!}}

friederbluemle · 2019-09-18T17:42:35Z

I think the tests are failing.. How do I regenerate the test fixtures @brodybits?

brody4hire · 2019-09-18T17:49:19Z

I think the tests are failing.. How do I regenerate the test fixtures @brodybits?

do npx jest -u or yarn jest -u

and I have some feedback coming up

thanks @friederbluemle for the nice contribution!

friederbluemle · 2019-09-18T18:18:59Z

Thanks @brodybits - Updated and rebased.

templates/android.js

brody4hire

Really nice, overall

I think there are some changes that should be in separate PRs, which I have already raised. I would be happy to merge the separate PRs in order to help unblock this proposal.

I also raised some comments on some items that I don't really understand here.

@friederbluemle I would also really appreciate it if you would update the title and probably the commit message to better describe the actual change. The purpose is to help people like myself, evidently not a Maven or Gradle expert, to understand what the change does and why.

I will now respond to the recent comments from @friederbluemle.

templates/android.js

brody4hire · 2019-09-18T18:45:02Z

templates/android.js

    maven {
        // All of React Native (JS, Obj-C sources, Android binaries) is installed from npm
-        // Matches recent template from React Native 0.59 / 0.60


Why remove the comments here?

brody4hire · 2019-09-18T18:45:45Z

templates/android.js

-        // https://github.com/facebook/react-native/blob/0.59-stable/template/android/build.gradle#L30
-        // https://github.com/facebook/react-native/blob/0.60-stable/template/android/build.gradle#L28
-        url "$projectDir/../node_modules/react-native/android"
+        url "$rootDir/../node_modules/react-native/android"


additional comment would be nice for people like me who evidently don't understand Gradle and Maven so well

These changes are taken from the official RN template. I did not make any modification, since this is the file that will end up in an end user's generated project. I did not see any comments or reasons for this to be different here. If additional documentation/comments are necessary I'd suggest we go to the source (react-native) first to update it there.

brody4hire · 2019-09-18T18:46:21Z

templates/android.js

    }
-    mavenCentral()
+    google()


Looks like there is some reordering. Explanation would be helpful for people like me.

tests/integration/cli/create/view/__snapshots__/cli-create-with-view.test.js.snap

friederbluemle · 2019-09-18T20:23:06Z

@brodybits - As you requested, I lowered the Android Gradle plugin version to 3.4.1 again. We can make the update separately. What do you suggest as a PR title and commit message subject line?

This wraps the buildscripts dependencies in a conditional. More information here: facebook/react-native#25569 (comment)

friederbluemle · 2019-09-18T21:06:10Z

Resolved conflicts and rebased.

brody4hire

I would appreciate it if we could fix a few more things before merging:

Let me test and merge PR [tbd ???] update Android build tools Gradle classpath #139, to cleanup and update the Android build tools Gradle classpath item, and update this proposal accordingly
Link to a new issue somewhere that describes the solution you proposed in Update build.gradle facebook/react-native#25569 (comment), like I requested in Add gradle plugin and androidX versions into project-level gradle react-native-community/discussions-and-proposals#151 (comment), without all of the surrounding noise.
Some of your answers came with explanations and links that I think are really important to help people like me understand what is going on in the Gradle file. I would be grateful if you could help people like me by including this information in comments in the Gradle file. Otherwise I would probably want to do this in a followup PR.

~~One step further may be for you to open a new, clean PR with your proposed solution, after I have finished testing and merging PR #139.~~

I am sorry for all of this holdup, I just want to get this update as clean and understandable as possible for the sake of people like myself.

brody4hire

I just tested the following cases:

generate a view module with example using [email protected] (default) & run the example on Android
generate a non-view module with example using [email protected] (default) & run the example on Android
generate a non-view module with example using react-native@latest (0.60) & run the example on Android
generate a non-view module with example using react-native@next (0.61-rc) & run the example on Android

followup items will be tracked in a new issue, merging now

thanks again to @friederbluemle for the nice work!

brody4hire · 2019-09-19T14:48:00Z

I just published version 0.11.0 with this update included.

I think some followup fixes will be needed, will raise one or more issues for the tracking purposes.

/cc @SaeedZhiany

SaeedZhiany · 2019-09-20T11:06:25Z

Wonderful update!
Thanks, @friederbluemle, and @brodybits.

I think some followup fixes will be needed

What kind of followup is needed?

brody4hire · 2019-09-20T14:54:21Z

What kind of followup is needed?

see #143

as a followup to updates from PR #135

as a followup to updates from PR #135 * move the other recommended references * remove react-native template reference no longer needed

friederbluemle mentioned this pull request Sep 18, 2019

[wip/tbd] remove Android Gradle tools reference #129

Closed

brody4hire self-assigned this Sep 18, 2019

brody4hire mentioned this pull request Sep 18, 2019

no sudo in generated doc for Android #136

Merged

friederbluemle force-pushed the update-project branch from adf42e4 to 4b1acb6 Compare September 18, 2019 18:18

brody4hire reviewed Sep 18, 2019

View reviewed changes

templates/android.js Outdated Show resolved Hide resolved

brody4hire reviewed Sep 18, 2019

View reviewed changes

templates/android.js Show resolved Hide resolved

This was referenced Sep 18, 2019

cleanup generated android/build.gradle artifact #138

Merged

[tbd ???] update Android build tools Gradle classpath #139

Draft

brody4hire reviewed Sep 18, 2019

View reviewed changes

friederbluemle force-pushed the update-project branch from 4b1acb6 to 88df273 Compare September 18, 2019 20:22

friederbluemle force-pushed the update-project branch from 88df273 to 066c796 Compare September 18, 2019 20:23

Update android build.gradle template setup

40afc0e

This wraps the buildscripts dependencies in a conditional. More information here: facebook/react-native#25569 (comment)

friederbluemle force-pushed the update-project branch from 066c796 to 40afc0e Compare September 18, 2019 21:06

brody4hire mentioned this pull request Sep 18, 2019

Add gradle plugin and androidX versions into project-level gradle react-native-community/discussions-and-proposals#151

Closed

brody4hire reviewed Sep 18, 2019

View reviewed changes

brody4hire approved these changes Sep 19, 2019

View reviewed changes

brody4hire merged commit 453bc19 into brody4hire:dev Sep 19, 2019

brody4hire mentioned this pull request Sep 19, 2019

[Android Studio support] Gradle build issue after creating. #120

Closed

friederbluemle deleted the update-project branch September 19, 2019 15:09

brody4hire pushed a commit that referenced this pull request Sep 22, 2019

add references & links to generated build.gradle

fea4c6b

as a followup to updates from PR #135

This was referenced Sep 22, 2019

add references & links to generated build.gradle #152

Closed

[wip (tbd)] fix react-native reference comments in build.gradle #154

Draft

brody4hire pushed a commit that referenced this pull request Nov 15, 2019

add references & links to generated build.gradle (see PR #152)

b4a3192

as a followup to updates from PR #135 * move the other recommended references * remove react-native template reference no longer needed

brody4hire mentioned this pull request Feb 10, 2021

fix(cleanup): remove extra plugin items from build.gradle #402

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update android build.gradle template setup #135

Update android build.gradle template setup #135

friederbluemle commented Sep 18, 2019 •

edited

Loading

friederbluemle commented Sep 18, 2019

brody4hire commented Sep 18, 2019

friederbluemle commented Sep 18, 2019

brody4hire left a comment

brody4hire Sep 18, 2019

brody4hire Sep 18, 2019

friederbluemle Sep 18, 2019

brody4hire Sep 18, 2019

friederbluemle commented Sep 18, 2019

friederbluemle commented Sep 18, 2019

brody4hire left a comment •

edited

Loading

brody4hire left a comment

brody4hire commented Sep 19, 2019

SaeedZhiany commented Sep 20, 2019

brody4hire commented Sep 20, 2019

Update android build.gradle template setup #135

Update android build.gradle template setup #135

Conversation

friederbluemle commented Sep 18, 2019 • edited Loading

friederbluemle commented Sep 18, 2019

brody4hire commented Sep 18, 2019

friederbluemle commented Sep 18, 2019

brody4hire left a comment

Choose a reason for hiding this comment

brody4hire Sep 18, 2019

Choose a reason for hiding this comment

brody4hire Sep 18, 2019

Choose a reason for hiding this comment

friederbluemle Sep 18, 2019

Choose a reason for hiding this comment

brody4hire Sep 18, 2019

Choose a reason for hiding this comment

friederbluemle commented Sep 18, 2019

friederbluemle commented Sep 18, 2019

brody4hire left a comment • edited Loading

Choose a reason for hiding this comment

brody4hire left a comment

Choose a reason for hiding this comment

brody4hire commented Sep 19, 2019

SaeedZhiany commented Sep 20, 2019

brody4hire commented Sep 20, 2019

friederbluemle commented Sep 18, 2019 •

edited

Loading

brody4hire left a comment •

edited

Loading