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

[flutter_plugin_tools] Validate pubspec description #4396

Merged
merged 4 commits into from
Sep 30, 2021
Merged

[flutter_plugin_tools] Validate pubspec description #4396

merged 4 commits into from
Sep 30, 2021

Conversation

stuartmorgan-g
Copy link
Contributor

pub.dev deducts points for having a pubspec.yaml description that is too short or too long; several of our plugins are losing points on this. To ensure that we are following—and modeling—best practices, this adds a check that our description fields meet pub.dev expectations.

Fixes our existing violations. Two are not published even though this only takes effect once published:

  • camera: We change this plugin pretty frequently, so this should go out soon without adding a release just for this trivial issue.
  • wifi_info_flutter: This is deprecated, so we don't plan to release it. It has to be fixed to allow the tool change to land though.

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.
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Sep 29, 2021
mvanbeusekom
mvanbeusekom approved these changes Sep 30, 2021
View reviewed changes
Copy link
Contributor

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM, I have found one small typing mistake in one of the comments but otherwise looks very good.

@stuartmorgan-g @mvanbeusekom
Typo fix
dd6d0d2 
Co-authored-by: Maurits van Beusekom <[email protected]>
ditman
ditman approved these changes Sep 30, 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.

LGTM! Just a minor comment about detecting the implementation pkgs.

script/tool/lib/src/common/repository_package.dart
@@ -58,6 +58,15 @@ class RepositoryPackage {
bool get isPlatformInterface =>
directory.basename.endsWith('_platform_interface');

/// True if this appears to be a platform implementation package, according to
/// repository conventions.
bool get isPlatformImplementation =>
Copy link
Member

Choose a reason for hiding this comment

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

Should this be determined by the presence of the implements pubspec.yaml property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we ever find that this heuristic isn't working, that's the way to go (and by having it centralized here was can easily improve it later). That's way more involved though since it requires loading and parsing the pubspec, and I suspect the heuristic may well be all we ever need.

@stuartmorgan-g stuartmorgan-g merged commit cf40966 into flutter:master Sep 30, 2021
@stuartmorgan-g stuartmorgan-g deleted the description-check branch September 30, 2021 17:30
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 30, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 30, 2021
@engine-flutter-autoroll
cf40966 [flutter_plugin_tools] Validate pubspec description (flutter/…
b3264bf 
…plugins#4396)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 30, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 30, 2021
@engine-flutter-autoroll
cf40966 [flutter_plugin_tools] Validate pubspec description (flutter/…
58c7c69 
…plugins#4396)
NickalasB added a commit to NickalasB/plugins that referenced this pull request Sep 30, 2021
@NickalasB
Merge branch 'master' into add_zoom_to_android_webview
bdb07f7 
* master:
  [google_maps_flutter] Add Marker drag events (flutter#2838)
  [flutter_plugin_tools] Validate pubspec description (flutter#4396)
  Add file_selector to the repo list (flutter#4395)
  [in_app_purchase] Fix in_app_purchase_android/README.md (flutter#4363)
  [google_maps_flutter_web] Add Marker drag events (flutter#4385)
  [webview_flutter] Fixed todos in FlutterWebView.java (flutter#4394)
  Handle `PurchaseStatus.restored` correctly in example. (flutter#4393)
  Handle restored purchases in iOS example app (flutter#4392)
  [file_selector] Remove custom analysis options (flutter#4382)
  [flutter_plugin_tools] Check licenses in Kotlin (flutter#4373)
  Fixed _CastError when running example App (flutter#4390)
  [in_app_purchase] Ensure the introductoryPriceMicros field is transported as a String. (flutter#4370)
  Load navigation controls immediately. (flutter#4377)
  [camera] Fix IllegalStateException being thrown in Android implementation when switching activities. (flutter#4319)

# Conflicts:
#	packages/webview_flutter/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/webview_flutter_android/CHANGELOG.md
mgonzalezc pushed a commit to mgonzalezc/plugins that referenced this pull request Oct 12, 2021
Merge branch 'master' into quick_actions_mindapps
419b195 
* master: (1126 commits)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
  [webview_flutter] Update webview platform interface with new methods for running JavaScript. (flutter#4401)
  [webview_flutter] Add zoomEnabled to webview flutter platform interface (flutter#4404)
  [ci] Remove obsolete Dockerfile (flutter#4405)
  Fix order-dependant platform interface tests (flutter#4406)
  [google_maps_flutter]: LatLng longitude loses precision in constructor #90574 (flutter#4374)
  [google_maps_flutter] Add Marker drag events (flutter#2838)
  [flutter_plugin_tools] Validate pubspec description (flutter#4396)
  Add file_selector to the repo list (flutter#4395)
  [in_app_purchase] Fix in_app_purchase_android/README.md (flutter#4363)
  [google_maps_flutter_web] Add Marker drag events (flutter#4385)
  [webview_flutter] Fixed todos in FlutterWebView.java (flutter#4394)
  Handle `PurchaseStatus.restored` correctly in example. (flutter#4393)
  Handle restored purchases in iOS example app (flutter#4392)
  [file_selector] Remove custom analysis options (flutter#4382)
  [flutter_plugin_tools] Check licenses in Kotlin (flutter#4373)
  Fixed _CastError when running example App (flutter#4390)
  [in_app_purchase] Ensure the introductoryPriceMicros field is transported as a String. (flutter#4370)
  ...

# Conflicts:
#	packages/quick_actions/ios/Classes/FLTQuickActionsPlugin.m
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
@stuartmorgan-g @amantoux
[flutter_plugin_tools] Validate pubspec description (flutter#4396)
e69b50b 
pub.dev deducts points for having a pubspec.yaml `description` that is too short or too long; several of our plugins are losing points on this. To ensure that we are following—and modeling—best practices, this adds a check that our `description` fields meet pub.dev expectations.

Fixes our existing violations. Two are not published even though this only takes effect once published:
- camera: We change this plugin pretty frequently, so this should go out soon without adding a release just for this trivial issue.
- wifi_info_flutter: This is deprecated, so we don't plan to release it. It has to be fixed to allow the tool change to land though.
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
@stuartmorgan-g @KyleFin
[flutter_plugin_tools] Validate pubspec description (flutter#4396)
0fd2d6f 
pub.dev deducts points for having a pubspec.yaml `description` that is too short or too long; several of our plugins are losing points on this. To ensure that we are following—and modeling—best practices, this adds a check that our `description` fields meet pub.dev expectations.

Fixes our existing violations. Two are not published even though this only takes effect once published:
- camera: We change this plugin pretty frequently, so this should go out soon without adding a release just for this trivial issue.
- wifi_info_flutter: This is deprecated, so we don't plan to release it. It has to be fixed to allow the tool change to land though.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ditman ditman ditman approved these changes

@mvanbeusekom mvanbeusekom mvanbeusekom approved these changes

@gaaclarke gaaclarke Awaiting requested review from gaaclarke

@bparrishMines bparrishMines Awaiting requested review from bparrishMines

Assignees
No one assigned
Labels
cla: yes p: camera p: espresso p: file_selector p: plugin_platform_interface p: wifi_info_flutter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stuartmorgan-g @ditman @mvanbeusekom