-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[tool] Consider comment-only changes to be dev-only #4279
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
[tool] Consider comment-only changes to be dev-only #4279
Conversation
Updates the state checker to inspect changed Dart files to see if the only changes are implementation (not documentation) comment lines. In particular, this will fix the problem of CI flagging changes that do nothing but add `// ignore:` comments (for federated package changes involving deprecation, framework changes that require temporary ignores in packages to support stable, etc.) as needing version and changelog changes.
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.
Seems like a good time for a trailing comma. Do we have style guidelines for this?
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.
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use-a-trailing-comma-for-arguments-parameters-and-list-items-but-only-if-they-each-have-their-own-line seems like it covers it. The style rules there don't always apply 100% since we use the auto-formatter, but in this case it's both better under the autoformatter and follows the style guide so it's a win all around :)
I definitely don't remember to use trailing commas as much as I should.
82574ff
to
6002a3a
Compare
auto label is removed for flutter/packages, pr: 4279, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto label is removed for flutter/packages, pr: 4279, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter/packages@d041934...6b70804 2023-06-23 [email protected] [go_router] Reduces excessive rebuilds due to inherited look up. (flutter/packages#4227) 2023-06-23 [email protected] [image_picker] Update to 1.0 (flutter/packages#4285) 2023-06-23 [email protected] [tool] Consider comment-only changes to be dev-only (flutter/packages#4279) 2023-06-23 [email protected] [ci] Switch to LUCI for Android build-all (flutter/packages#4274) 2023-06-23 [email protected] [file_selector] Add file group to save return value (flutter/packages#4222) 2023-06-23 [email protected] [go_router] Adds onException to GoRouter constructor. (flutter/packages#4216) 2023-06-23 [email protected] [file_selector] Add file group to save return value - implementations (flutter/packages#4273) 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 [email protected],[email protected] 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
Updates the state checker to inspect changed Dart files to see if the only changes are implementation (not documentation) comment lines. In particular, this will fix the problem of CI flagging changes that do nothing but add
// ignore:
comments (for federated package changes involving deprecation, framework changes that require temporary ignores in packages to support stable, etc.) as needing version and changelog changes