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

[flutter_plugin_tools] Add --packages, and deprecated --plugins #4134

Merged
merged 1 commit into from Jul 4, 2021
Merged

[flutter_plugin_tools] Add --packages, and deprecated --plugins #4134

merged 1 commit into from Jul 4, 2021

Conversation

stuartmorgan
Copy link
Contributor

Most of the tool operates on packages in general, and the targetting
done currently by the --plugins flag is not actually restricted to
plugins, so this makes the name less confusing.

--plugin will continue to work for now to minimize disruption to
scripts and workflows.

Part of flutter/flutter#83413

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. (Note that 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.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • [x ] All existing and new tests are passing.

@stuartmorgan
[flutter_plugin_tools] Add --packages, and deprecated --plugins
490af91 
Most of the tool operates on packages in general, and the targetting
done currently by the `--plugins` flag is not actually restricted to
plugins, so this makes the name less confusing.

Part of flutter/flutter#83413
@stuartmorgan stuartmorgan requested a review from ditman July 2, 2021 18:20
@google-cla google-cla bot added the cla: yes label Jul 2, 2021
ditman
ditman approved these changes Jul 2, 2021
View reviewed changes
Copy link
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.

This looks good to me. I'd try to add a deprecation notice if the user calls the script with --plugins instead of --packages so they're aware of this change, but if it's not a straightforward change, I wouldn't use too much time.

(I did a quick google search of "--plugins" in flutter.dev and our github and couldn't find any docs mentioning the argument? odd)

script/tool/lib/src/common/plugin_command.dart
splitCommas: true,
help:
'Specifies which plugins the command should run on (before sharding).',
valueHelp: 'plugin1,plugin2,...',
'Specifies which packages the command should run on (before sharding).\n',
Copy link
Member

Choose a reason for hiding this comment

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

Should we note in this help text that this is deprecating _pluginsArg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The set of people likely to actually be using this tool manually is very small; I'm not worried about them getting the memo.

@stuartmorgan
Copy link
Contributor Author

(I did a quick google search of "--plugins" in flutter.dev and our github and couldn't find any docs mentioning the argument? odd)

I'd be pretty surprised if anything on flutter.dev mentioned it. We should add the package-targeting options to the tool README at some point though.

@stuartmorgan stuartmorgan merged commit f466ccc into flutter:master Jul 4, 2021
@stuartmorgan stuartmorgan deleted the rename-plugins-flag branch July 4, 2021 16:26
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 4, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2021
@engine-flutter-autoroll
f466ccc [flutter_plugin_tools] Add --packages, and deprecated --plugi…
c2d1122 
…ns (flutter/plugins#4134)
fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request Jul 4, 2021
@engine-flutter-autoroll
f466ccc [flutter_plugin_tools] Add --packages, and deprecated --plugi…
fed99b9 
…ns (flutter/plugins#4134) (#85866)
amantoux pushed a commit to amantoux/plugins that referenced this pull request Jul 10, 2021
@stuartmorgan @amantoux
[flutter_plugin_tools] Add --packages, and deprecated --plugins (flut…
28ab959 
…ter#4134)

Most of the tool operates on packages in general, and the targetting
done currently by the `--plugins` flag is not actually restricted to
plugins, so this makes the name less confusing.

Part of flutter/flutter#83413
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@stuartmorgan @fotiDim
[flutter_plugin_tools] Add --packages, and deprecated --plugins (flut…
b39b2e6 
…ter#4134)

Most of the tool operates on packages in general, and the targetting
done currently by the `--plugins` flag is not actually restricted to
plugins, so this makes the name less confusing.

Part of flutter/flutter#83413
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ditman ditman ditman approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
2 participants
@stuartmorgan @ditman