Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tools] Fix subpackage analysis#5027

Merged
fluttergithubbot merged 2 commits intoflutter:mainfrom
stuartmorgan-g:tool-analyze-subpackage-fix
Mar 11, 2022
Merged

[flutter_plugin_tools] Fix subpackage analysis#5027
fluttergithubbot merged 2 commits intoflutter:mainfrom
stuartmorgan-g:tool-analyze-subpackage-fix

Conversation

@stuartmorgan-g
Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g commented Mar 9, 2022

Fixes a regression in analyze when packages have non-example
subpackages. Restructuring in 0.8.0 accidentally caused them not to have
their dependencies fetched, causing analysis failures.

Unblocks rolling flutter/packages to the latest tooling.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@stuartmorgan-g
[flutter_plugin_tools] Fix subpackage analysis
81cc512 
Fixes a regression in `analyze` when packages have non-example
subpackages. Restructuring in 0.8.0 accidentally caused them not to have
their dependencies fetched, causing analysis failures.

Unblocks rolling flutter/packages to the latest tooling.
@stuartmorgan-g
Copy link
Copy Markdown
Contributor Author

stuartmorgan-g commented Mar 9, 2022 •
edited
Loading

I was thinking analysis was a run-on-packages-and-subpackages iteration when I made the previous change; I forgot that it's actually iterating over just the top-level packages, but that analyze automatically runs over subpackages within the directory.

@ditman
Copy link
Copy Markdown
Member

ditman commented Mar 10, 2022 

./script/tool_runner.sh make-deps-path-based --target-dependencies-with-non-breaking-updates
Finding changed packages relative to "6852d0a311a8c501b1718ca54b382b97c4db1a22"...
Rewriting references to: flutter_plugin_tools...
�[31mUnable to find package "flutter_plugin_tools"�[0m

Exit status: 3

@stuartmorgan-g
Copy link
Copy Markdown
Contributor Author

stuartmorgan-g commented Mar 10, 2022

Doh

@stuartmorgan-g
Copy link
Copy Markdown
Contributor Author

stuartmorgan-g commented Mar 10, 2022

Should be fixed now; I added logic to skip packages that aren't in the packages directory when finding changed packages to rewrite.

@ditman
Copy link
Copy Markdown
Member

ditman commented Mar 10, 2022

Ah, good point, I didn't remember that tools was outside of packages (x_x)

ditman
ditman approved these changes Mar 10, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 LGTM! 🛸

Comment thread script/tool/lib/src/analyze_command.dart
.pubspecFile
.existsSync()) {
final int exitCode = await processRunner.runAndStream(
flutterCommand, <String>['packages', 'get'],
Copy link
Copy Markdown
Member

@ditman ditman Mar 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is extremely nitpicky, I'm sure I've not noticed it in dozens of other PRs!, but flutter packages is an undocumented alias (kept for backwards compatibility, IIRC) of: flutter pub 🤓

$ flutter --help | grep packages
  pub               Commands for managing Flutter packages.

(It doesn't show up even in -v mode of help!, but there's a --packages that has a different purpose.)

Copy link
Copy Markdown
Member

@ditman ditman Mar 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(flutter packages get will probably work forever, but I'm flagging it just in case 😛)

Copy link
Copy Markdown
Contributor Author

@stuartmorgan-g stuartmorgan-g Mar 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL! I'll do a PR to fix that shortly.

@stuartmorgan-g stuartmorgan-g added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 11, 2022
@fluttergithubbot fluttergithubbot merged commit d363894 into flutter:main Mar 11, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 14, 2022
@engine-flutter-autoroll
d3638944b [flutter_plugin_tools] Fix subpackage analysis (flutter/plu…
d8ce43b 
…gins#5027)
@stuartmorgan-g stuartmorgan-g deleted the tool-analyze-subpackage-fix branch April 19, 2022 17:35
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
@stuartmorgan-g @mauricioluz
[flutter_plugin_tools] Fix subpackage analysis (flutter#5027)
158685a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bparrishMines bparrishMines Awaiting requested review from bparrishMines

1 more reviewer

@ditman ditman ditman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stuartmorgan-g @ditman @fluttergithubbot