Skip to content
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

Applied missing changes from bumping Gradle wrapper to 6.0.1 #27639

Closed

Conversation

@SaeedZhiany
Copy link
Contributor

SaeedZhiany commented Dec 30, 2019

Summary

This PR is related to #27290.
I just upgraded my project's Gradle wrapper version to 6.0.1 and I realized some files have some differences with the files in react-native template folder. so I create this PR to apply differences.
the main difference is in the gradlew file. I'm not familiar with Linux shell scripts but it seems there was a syntax error in case items syntax. ( should not be used in declaring case's items. it may has building error in Linux OS.

Changelog

[Android] [Fixed] - Applied missing changes from bumping Gradle wrapper to 6.0.1

Test Plan

I have no Linux OS right now, so I can't directly test these changes, but because the changes have made by running gradlew wrapper command, it should not break CI. (I hope :) )

@SaeedZhiany

This comment has been minimized.

Copy link
Contributor Author

SaeedZhiany commented Dec 30, 2019

In ci/circleci: test_android's log, it seems it can build Android RNTester App successfully and it failed in Collect Test Results phase because some directory has not been found.

@kelset kelset requested a review from dulmandakh Jan 7, 2020
Copy link
Contributor

mikehardy left a comment

This is standard - any time you bump gradle you have to commit the full set of changes - the dependency in the gradle settings file, as well as the shell wrappers and the jar. Looks like some was just missed in moving gradle to the new version and this cleans that up? Which is easy to agree with

Copy link
Contributor

friederbluemle left a comment

👍 Yes, this should be merged. Gradle 6.0+ comes with these updated wrapper scripts. Most likely what happened is that a previous version (Gradle 5.0+) was used to upgrade Gradle wrapper to 6.0 (which of course did not contain the new scripts yet). Running the wrapper task using Gradle 6.0.1 locally yields zero changed lines against the PR head branch.

Btw, the case items are not a syntax error, but it is more conventional to omit the (.

@friederbluemle

This comment has been minimized.

Copy link
Contributor

friederbluemle commented Jan 10, 2020

In ci/circleci: test_android's log, it seems it can build Android RNTester App successfully and it failed in Collect Test Results phase because some directory has not been found.

Please check again after this PR has been merged. The error is most likely caused by something else.

Copy link
Collaborator

dulmandakh left a comment

LGTM. I use gradle wrapper to upgrade versions, like ./gradlew wrapper --gradle-version=6.0.1 --distribution-type=all, and it's strange that it didn't produce this change. Or I made a mistake. Sorry.

@dulmandakh dulmandakh requested a review from cpojer Jan 10, 2020
@cpojer
cpojer approved these changes Jan 10, 2020
Copy link
Contributor

cpojer left a comment

Thank you!

Copy link

facebook-github-bot left a comment

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@friederbluemle

This comment has been minimized.

Copy link
Contributor

friederbluemle commented Jan 10, 2020

LGTM. I use gradle wrapper to upgrade versions, like ./gradlew wrapper --gradle-version=6.0.1 --distribution-type=all, and it's strange that it didn't produce this change. Or I made a mistake. Sorry.

No worries :)

Tip for the future: When updating between major versions (or when updates to the wrapper scripts are expected), run the command twice: The first time ./gradlew it still the old version (unaware of the new scripts).

@react-native-bot

This comment has been minimized.

Copy link
Collaborator

react-native-bot commented Jan 13, 2020

This pull request was successfully merged by @SaeedZhiany in aa0ef15.

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

@SaeedZhiany SaeedZhiany deleted the SaeedZhiany:FixGradleWrapperFiles branch Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.