-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Conversation
In |
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 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
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.
👍 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 (
.
Please check again after this PR has been merged. The error is most likely caused by something else. |
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.
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.
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.
Thank you!
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.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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 |
This pull request was successfully merged by @SaeedZhiany in aa0ef15. When will my fix make it into a release? | Upcoming Releases |
…k#27639) Summary: This PR is related to facebook#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 Pull Request resolved: facebook#27639 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 :) ) Differential Revision: D19341671 Pulled By: cpojer fbshipit-source-id: ccfc3c12af3f5468671737e5ba0b1674b4491590
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 incase
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 :) )