-
Notifications
You must be signed in to change notification settings - Fork 29.7k
WIP Create .flutter-plugins file per platform #47258
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
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
71ed112 to
b9ea0c5
Compare
| system('rm -rf .symlinks') | ||
| system('mkdir -p .symlinks/plugins') | ||
| plugin_pods = parse_KV_file('../.flutter-plugins') | ||
| plugin_pods = parse_KV_file('./Flutter/.flutter-plugins') |
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.
The Podfiles aren't owned by Flutter, and they get generated only when the app is created. So existing Podfiles are going to expect '../.flutter-plugins' and won't get this change without a migration.
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.
Agreed. I am still generating the current file at the same location to avoid breaking existing apps. I'll add a warning message if they are using the "old" podfile so the user can delete it and auto-generate it again.
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.
Can you copy/make the symlinks in this Ruby file from the old one if the old directory is present and the new one isn't so they don't need to regenerate anything, and a pod install won't just fail? The unless block right above is doing a similar one-time cp migration
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.
Or something like this so they know what to do if the new file doesn't exist:
plugin_file = File.join(__dir__, 'Flutter', '.flutter-plugins')
unless File.exist?(plugin_file)
raise "Flutter/.flutter-plugins must exist. If you're running pod install manually, make sure flutter pub get is executed first"
end
plugin_pods = parse_KV_file(plugin_file)
Codecov Report
@@ Coverage Diff @@
## master #47258 +/- ##
==========================================
+ Coverage 63.03% 64.2% +1.17%
==========================================
Files 204 205 +1
Lines 20919 20777 -142
==========================================
+ Hits 13186 13340 +154
+ Misses 7733 7437 -296
Continue to review full report at Codecov.
|
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
53a65fe to
d66adc0
Compare
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
Re: "get fetches packages and injects plugin" failure: in |
|
Closing in favor of #49085. The changes to the templates will be done in a separate PR |
Description
Creates a
.flutter-pluginsand a.flutter-plugins-dependenciesfile for each platform project instead of in the Flutter root project for all platforms except for Android.Related Issues
#46618
Tests
Tests remaining
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read [Handling breaking changes].