-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[pigeon] Require analyzer 5.13.0, prepare for NamedType refactoring. #4127
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
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Or rather, the Dart SDK needs to be updated to 2.19. (And indeed, current versions of |
99ca398
to
12aa5a4
Compare
packages/pigeon/lib/pigeon_lib.dart
Outdated
@@ -1546,3 +1534,13 @@ ${_argParser.usage}'''; | |||
} | |||
} | |||
} | |||
|
|||
extension on dart_ast.NamedType { |
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.
is this temporary or permanent?
if it's temporary, please add a TODO explaining the timeline for when it will be removed
if it's permanent, please use a function instead of an extension. The use of Dart extension features violates our style guide due to its harm on readability (someone reading the code will look for "qualifiedName" on the dart_ast.NamedType
API and not find it; there's nothing to point them to this part of the code).
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.
It was not supposed to be temporary. So, I changed it to a private static method.
Let me know if you want it to be exactly a function instead of a static method.
test-exempt: code refactor with no semantic change |
It appears that the code will need to be formatted You will also need to update the version number in the pubspec.yaml file, as well as the generator_tools.dart file, and add to the changelog. |
df0f0d8
to
6924b9c
Compare
flutter/packages@a84b2c2...e13b8c4 2023-06-08 [email protected] [tool] Only run unit tests in Chrome for inline web (flutter/packages#4153) 2023-06-08 [email protected] [in_app_purchase] Make the _FeatureCard constructor const in the Android example app (flutter/packages#4162) 2023-06-08 [email protected] [shared_preferences] Fix initialization race (flutter/packages#4159) 2023-06-07 [email protected] [go_router] Refactors imperative APIs and browser history (flutter/packages#4134) 2023-06-07 [email protected] [various] Add `http` 1.0 compatibility (flutter/packages#4147) 2023-06-07 [email protected] [go_router_builder] Accept required parameters not in path (flutter/packages#4039) 2023-06-07 [email protected] Roll Flutter from 0b74153 to 8a5c22e (46 revisions) (flutter/packages#4160) 2023-06-07 [email protected] [pigeon] Require analyzer 5.13.0, prepare for NamedType refactoring. (flutter/packages#4127) 2023-06-07 [email protected] Roll Flutter (stable) from f92f441 to 682aa38 (1 revision) (flutter/packages#4157) 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
We would like to make a breaking change to the Dart analyzer.
https://dart-review.googlesource.com/c/sdk/+/303280
flutter/flutter#128212
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.