Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[all] Add topics to pubspecs #4771

Merged
merged 12 commits into from
Aug 29, 2023
Merged

[all] Add topics to pubspecs #4771

merged 12 commits into from
Aug 29, 2023

Conversation

stuartmorgan
Copy link
Contributor

Adds topics to all packages, supporting the new pub feature for categorizing packages. The heuristics I used were:

  • Try to use existing topics from https://pub.dev/topics where applicable
  • Add new topics as necessary to cover things that seemed like obvious relevant topics
  • Include the plugin name as a topic for all federated plugin packages, for grouping (since pub doesn't inherently group or cross-link implementations)

This is not an attempt to be exhaustive; as topics evolve I expect we will add more or adjust.

Also updates the repo tooling to enforce topics, so that we don't forget to add them to new packages. The enforced rule is:

  • All packages must have at least one topic. We could potentially change this to allow an empty topics section so that we are enforcing that we didn't just forget to add the section, but in practice even for packages that we don't expect people to be likely to use, I didn't have any issue coming up with at least one relevant topic.
  • Federated plugin packages must contain the plugin name as a topic.

While this isn't time-critical, I chose to include version bumps so that we aren't rolling out topics in a piecemeal way (e.g., with only a random subset of a federated plugin's packages having topics on pub.dev based on what has happened to have a bugfix).

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/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package 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.

yjbanov
yjbanov approved these changes Aug 25, 2023
View reviewed changes
packages/url_launcher/url_launcher_web/pubspec.yaml
topics:
- os-integration
- url-launcher
- urls
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it also make sense to add "link" or "links"? It's a more common term on the web compared to URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! I'll add it to all of the url_launcher packages.

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 fact that there's no consistency in topics between singular and plural makes me sad; hopefully that will shake out over time...)

Piinks
Piinks approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM!

gmackall
gmackall approved these changes Aug 25, 2023
View reviewed changes
Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

Topic choices for IAP, file selector, image picker, and url launcher LGTM

reidbaker
reidbaker approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

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

Thanks for the responses.

tarrinneal
tarrinneal approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@tarrinneal tarrinneal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

TSIOLFIAACSTUTYIYWTAT

packages/image_picker/image_picker/pubspec.yaml
- camera
- image-picker
- files
- file-selection
Copy link
Contributor

Choose a reason for hiding this comment

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

image/s, video/s ? At this point the package name is a little incorrect...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the one I probably feel the most conflicted about. I considered those, but the other packages that have image(s)/video(s) are about displaying them, so that seemed potentially confusing, and I oriented on the idea that it's about choosing files (just of specific kinds). I may have gone too far that way, but in many ways image_picker and file_selector are actually quite similar (and in fact on desktop, they are the same).

vashworth
vashworth approved these changes Aug 28, 2023
View reviewed changes
Copy link
Contributor

@vashworth vashworth left a comment

Choose a reason for hiding this comment

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

LGTM

cbracken
cbracken approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm. The small inconsistencies in pluralisation (files vs payment, for example) I assume are due to wanting to use existing topic tag where possible?

@stuartmorgan
Copy link
Contributor Author

Yes. I'm going to bring up with pub.dev folks whether we can do some server magic to merge things that differ only in pluralization during searches/queries.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Aug 29, 2023

auto label is removed for flutter/packages/4771, due to - The status or check suite Linux_web web_dart_unit_test_shard_1 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2023
@stuartmorgan
Copy link
Contributor Author

Landing manually while the last roll is still cycling, since this PR is extremely prone to collisions.

@stuartmorgan stuartmorgan merged commit b4985e2 into flutter:main Aug 29, 2023
71 checks passed
@stuartmorgan stuartmorgan removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2023
@stuartmorgan stuartmorgan deleted the topics branch August 29, 2023 19:10
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 30, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 30, 2023
@engine-flutter-autoroll
b4985e2 [all] Add topics to pubspecs (flutter/packages#4771)
9768760
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 30, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 30, 2023
@engine-flutter-autoroll
b4985e2 [all] Add topics to pubspecs (flutter/packages#4771)
2522e22
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 30, 2023
@engine-flutter-autoroll
Roll Packages from d7d3150 to 64af59e (4 revisions) (#133675)
d49eb4a 
flutter/packages@d7d3150...64af59e

2023-08-30 katelovett@google.com [two_dimensional_scrollables] Fix repaint boundary override in builder delegate (flutter/packages#4814)
2023-08-29 stuartmorgan@google.com [all] Add topics to pubspecs (flutter/packages#4771)
2023-08-29 engine-flutter-autoroll@skia.org Roll Flutter from ec387a4 to 6c95737 (24 revisions) (flutter/packages#4813)
2023-08-29 panuj330@gmail.com Fixed AlertDialog Height of example in image_picker (flutter/packages#4800)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbaker reidbaker mentioned this pull request Sep 15, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarrinneal tarrinneal tarrinneal approved these changes

@loic-sharma loic-sharma loic-sharma left review comments

@chunhtai chunhtai chunhtai approved these changes

@gmackall gmackall gmackall approved these changes

@Piinks Piinks Piinks approved these changes

@vashworth vashworth vashworth approved these changes

@jonahwilliams jonahwilliams jonahwilliams approved these changes

@gspencergoog gspencergoog gspencergoog approved these changes

@reidbaker reidbaker reidbaker approved these changes

@cbracken cbracken cbracken approved these changes

@yjbanov yjbanov yjbanov approved these changes

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

@hellohuanlin hellohuanlin Awaiting requested review from hellohuanlin hellohuanlin is a code owner

@LouiseHsu LouiseHsu Awaiting requested review from LouiseHsu LouiseHsu is a code owner

@camsim99 camsim99 Awaiting requested review from camsim99 camsim99 is a code owner

@ditman ditman Awaiting requested review from ditman ditman is a code owner

@MitchellGoodwin MitchellGoodwin Awaiting requested review from MitchellGoodwin MitchellGoodwin is a code owner

@bparrishMines bparrishMines Awaiting requested review from bparrishMines bparrishMines is a code owner

@Hixie Hixie Awaiting requested review from Hixie Hixie is a code owner

@dnfield dnfield Awaiting requested review from dnfield

@keyonghan keyonghan Awaiting requested review from keyonghan

@domesticmouse domesticmouse Awaiting requested review from domesticmouse domesticmouse is a code owner

@goderbauer goderbauer Awaiting requested review from goderbauer goderbauer is a code owner

Assignees
No one assigned
Labels
p: animations p: camera p: cross_file p: css_colors p: cupertino_icons p: espresso p: extension_google_sign_in_as_googleapis_auth p: file_selector p: flutter_adaptive_scaffold p: flutter_image p: flutter_lints p: flutter_markdown p: flutter_plugin_android_lifecycle p: flutter_template_images p: go_router_builder p: go_router p: google_maps_flutter p: google_sign_in p: image_picker p: in_app_purchase p: ios_platform_images p: local_auth p: metrics_center p: multicast_dns p: palette_generator p: path_provider p: pigeon p: plugin_platform_interface p: pointer_interceptor p: quick_actions p: rfw Remote Flutter Widgets p: shared_preferences p: standard_message_codec p: url_launcher p: video_player p: web_benchmarks p: webview_flutter p: xdg_directories platform-android platform-ios platform-linux platform-macos platform-web platform-windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.