[file_selector] Add file group to save return value - platform interface#4254
Conversation
|
This is unchanged from the main PR except for the last commit that adds temp ignores. |
78ab0e4 to
381716f
Compare
ditman
left a comment
There was a problem hiding this comment.
Minor comment about maybe making this a major version change.
| @@ -1,3 +1,7 @@ | |||
| ## 2.6.0 | |||
|
|
|||
| * Adds `getSaveLocation` and deprecates `getSavePath`. | |||
There was a problem hiding this comment.
If this were a major version bump, wouldn't it make this PR much smaller, without really affecting much further ones down the line? (next you'll implement and use the new method, and remove the ignores, but wouldn't it be ~the same amount of work to bump the minimum version of the dependency of this package in the others?)
There was a problem hiding this comment.
(Also, do the "ignores" need to be published to the changed packages?)
There was a problem hiding this comment.
Major version bumps to platform interfaces are unpleasant if there are any third-party implementations, and enabling those is one of the main goals of federation, so we try to minimize them.
There was a problem hiding this comment.
(Also, do the "ignores" need to be published to the changed packages?)
Nope, ignores are dev-only changes. I need to teach the tooling to do line-level analysis for the version and changelog checks, and ignore non-doc comments.
There was a problem hiding this comment.
Major version bumps to platform interfaces are unpleasant if there are any third-party implementations, and enabling those is one of the main goals of federation, so we try to minimize them.
As an alternative, can you deprecate getSavePath as a last change to the platform interface, once that every package has been updated to use the new version? Otherwise we're going to be yelling at users without them having an available solution, right?
There was a problem hiding this comment.
As an alternative, can you deprecate
getSavePathas a last change to the platform interface, once that every package has been updated to use the new version?
Our previous workflow was to in theory do it at the end, but in practice we just always forget and nothing gets deprecated :( And we don't even always remember to update all of our own code to use the new methods. So we're trying this flow instead to see how it goes.
Otherwise we're going to be yelling at users without them having an available solution, right?
Only users who are running analysis on our packages, which I would expect to be nobody.
There was a problem hiding this comment.
Our previous workflow was to in theory do it at the end, but in practice we just always forget and nothing gets deprecated :( And we don't even always remember to update all of our own code to use the new methods. So we're trying this flow instead to see how it goes.
git push --force then! :P
Platform interface portion of #4222
Part of flutter/flutter#107093
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).