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
Remove deprecated methods from BuildContext #69620
Conversation
packages/flutter/lib/fix_data.yaml
Outdated
@@ -0,0 +1,134 @@ | |||
# Spec file for "dart fix", see <insert URL>. |
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.
@bwilkerson Where can we link to here?
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.
We don't currently have a published version of the docs. They'll be the contents of the "User documentation" doc you've seen. I'd like to flesh them out a bit more before publishing them, but if we need them to be published sooner than that let me know.
Any additional feedback you have on the doc as a result of using it while writing this file would be much appreciated. (With thanks for your previous comment about adding some complete examples.)
- title: 'Rename to dependOnInheritedElement' | ||
date: 2019-11-22 | ||
element: | ||
uris: [ 'material.dart', 'cupertino.dart', 'widgets.dart' ] |
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.
I can see that coming up with the correct list here will be difficult. It will be easy to miss a lib that exports the symbol...
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.
That's a valid concern.
The design tradeoff is that we have less information that we can use to ensure that we're selecting the correct change in the case where a declaration has been removed. Having the URIs doesn't absolutely guarantee that we won't do the wrong thing, but it does make it significantly less likely. Unfortunately, I don't have concrete numbers for how often we'd do the wrong thing (either with or without them), but I am concerned about the effect on user trust if the tool makes incorrect changes.
/cc @Piinks |
We have to come up with a way to test these fixes. @bwilkerson Have you already given testing these definitions some thoughts? |
changes: | ||
- kind: 'rename' | ||
newName: 'dependOnInheritedElement' | ||
- title: 'Migrate to dependOnInheritedWidgetOfExactType' |
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.
As a drive by comment, you may want liberal use of whitespace and #
comments to help separate the individual refactors. This file will likely get pretty large and will be tough to navigate w/o some visual separation between fixes.
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.
Agreed and done.
version: 1 | ||
transforms: | ||
# Change made in https://github.com/flutter/flutter/pull/44189. | ||
- title: 'Rename to dependOnInheritedElement' |
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.
Unless the distinction is important to users we ought to be consistent in the naming of these actions. Consider changing "Rename" to "Migrate".
@goderbauer do you want to move forward with this change? |
The tests that need to be added here are almost the same as #72903 |
Yes, I'll update this PR. |
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. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
||
# Change made in https://github.com/flutter/flutter/pull/44189. | ||
- title: 'Migrate to ancestorRenderObjectOfType' | ||
date: 2019-11-22 |
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.
Oooh I see, this is the date of the deprecation? (#44189 here?)
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.
This pull request is not suitable for automatic merging in its current state.
|
Google tests have been fixed, this has a merge conflict now though. Strange |
Merge conflict is resolved. |
This pull request is not suitable for automatic merging in its current state.
|
Description
This removes the deprecated
BuildContext
methods that has reached the EOL . See table below:Similar methods exist on the
Element
class and will be removed from there in #72903.Part of deprecations that are slated for removal for next release in https://flutter.dev/go/deprecation-lifetime
More context: https://medium.com/flutter/deprecation-lifetime-in-flutter-e4d76ee738ad
Related Issues
Deprecated in #44189
Tests
Tests for quick fix tool.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.