Remove deprecated updateSemantics APIs.
#37396
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. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
a-wallen
force-pushed
the
remove_deprecated_updateSemantics
branch
from
a34c6d5
to
fe7fc58
Compare
a-wallen
force-pushed
the
remove_deprecated_updateSemantics
branch
from
fe7fc58
to
2edb658
Compare
| /// | ||
| /// In either case, this function disposes the given update, which means the | ||
| /// semantics update cannot be used further. | ||
| @Deprecated(''' |
There was a problem hiding this comment.
Just checking: has this been deprecated for a year? It looks like the deprecation notice was just added a month ago.
https://docs.flutter.dev/resources/compatibility#deprecation-policy
There was a problem hiding this comment.
No this has not been deprecated for over a year. That's a good catch. Thanks, Greg!
Is the solution to let the PR sit until this year passes?
|
No, but I'm surprised that the deprecation notice doesn't include the version that the deprecation first appeared in. We have a lint for that on the Framework side. The deprecation notice should look like this. Once it has the version number in it, then we'll be able to programmatically detect that it is a year old, and we'll remove it at that time (this is the kind of thing that @Piinks has been doing recently). Kate, are we supposed to also have deprecation lint checks (for deprecation message format compliance) in the engine |
There was a problem hiding this comment.
How long has this been deprecated for?
The analyzer-enforced deprecation format only applies in flutter/flutter right now I think. Same for the deprecation policy, we have not applied it to flutter/engine.
We have been discussing expanding to other packages like dart:ui , but since we haven't yet we probably shouldn't remove this right now. Especially if it has not been announced or had a migration guide generated yet.
|
@Piinks I think we should probably extend the deprecation notice checks to the |
Totally, that is something we have been discussing. It is just part of the custom analyzer script that runs on CI. |
|
Gold has detected about 2 new digest(s) on patchset 2. |
fixes flutter/flutter#112221
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
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.