-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Make consistent use of gradle wrapper #10993
Make consistent use of gradle wrapper #10993
Conversation
lgtm, works great after the last commit |
IMHO the fact that we keep having to update our apps to match new templates (we do this frequently) suggests a fundamental problem with our approach. Things that should be under our control should be in our repo, not in our developer's repo. Things that should be under their control should not be things we should ever try to update. |
@Hixie AFAIK, we have mostly been updating the apps in the Flutter repo to keep them close to what is generated by I agree it would be great to have a simple, operational rule to separate what should be in the Flutter repo and what should be in users' repos. Currently, we are not following a rule, but instead searching for a reasonable answer to the following set of design constraints and goals (partial list):
|
It's #5 and #8 that make this particularly tricky, I think. If we didn't have #8, we could construct the android/ and ios/ directories on the fly in the build/ directory, but that wouldn't satisfy #8. |
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
@@ -8,6 +8,6 @@ | |||
/captures | |||
GeneratedPluginRegistrant.java | |||
|
|||
/gradle | |||
gradle-wrapper.jar |
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.
We really shouldn't exclude the gradle wrapper files (including the .jar), if we want this to serve as an example of what an app should look like.
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 a clash between items 6 and 7 above.
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.
I've added a comment to the .gitignore
files explaining why they are excluded in the examples, and that it is not to be emulated.
@Hixie, note that 5./8. isn't only for writing specific code, or doing hybrid UI, it's also for the steps required to actually complete a release-version of an app. As you can see in https://flutter.io/android-release/ and https://flutter.io/ios-release/, that requires editing a number of files in the |
6fbacc4
to
6e70aaa
Compare
Fixes #10839 |
It seems you had removed the |
examples/
anddev/
apps with the Gradle wrapper without committing wrapper scripts and binaries to the Flutter repo.examples/
anddev/
apps have been updated to match new project templates.Fixes #9765.