-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New isSemantics and deprecate containsSemantics #180538
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
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.
Code Review
This pull request refactors the semantics testing matchers by introducing a new isSemantics matcher and deprecating the existing containsSemantics matcher. The isSemantics function is a rename of containsSemantics to better align with Dart's matcher naming conventions. The deprecated containsSemantics now delegates to isSemantics for backward compatibility. All usages across the test suite have been updated to the new isSemantics matcher.
|
Thanks for going the extra mile! Could you also add an automated fix for this migration? You'll need to add or edit a YAML file in this directory: https://github.com/flutter/flutter/tree/master/packages/flutter_test/lib/fix_data/fix_flutter_test Here are the instructions for a renamed function: https://github.com/flutter/flutter/blob/master/docs/contributing/Data-driven-Fixes.md#rename-a-method |
60fe9a6 to
791c162
Compare
Done. |
89be0f2 to
38d522b
Compare
38d522b to
a671104
Compare
packages/flutter_test/test_fixes/flutter_test/matchers/semantics.dart.expect
Outdated
Show resolved
Hide resolved
chunhtai
left a comment
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.
LGTM, according to our breaking change policy, you also need to add a migration guide in here https://github.com/flutter/website/tree/main/src/content/release/breaking-changes
Here is the policy just FYI https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
|
Google testing failures look unrelated to this change. Rerunning... |
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.
Thanks for cleaning this up, excellent work!
|
I think we need to wait for the migration guide to land first |
|
oh nvm, looks like the breaking change policy says the land the change first |
PR flutter/website#12891 created. |
_Issues fixed by this PR (if any):_ flutter/flutter#180534 _PRs or commits this PR depends on (if any):_ flutter/flutter#180538 cc @loic-sharma @chunhtai ## Presubmit checklist - [X] If you are unwilling, or unable, to sign the CLA, even for a _tiny_, one-word PR, please file an issue instead of a PR. - [X] If this PR is not meant to land until a future stable release, mark it as draft with an explanation. - [X] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style)—for example, it doesn't use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first-person pronouns). - [X] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Fix #180534
Pre-launch Checklist
///).