-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[url_launcher] Fix breaking change version conflict #4000
[url_launcher] Fix breaking change version conflict #4000
Conversation
stuartmorgan-g
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.
The fix here is correct (there's an alternate option of making this actually backward compatible down to 2.0.0 again by changing the function variable type, but getting more people onto url_launcher_platform_interface 2.0.3 is a better outcome since 2.0.0-2 don't work correctly with master), but the description in the PR and the CHANGELOG are not, per #3954 (comment), unless I've missed something.
|
@stuartmorgan So you think this PR would be fine to merge if the CHANGELOG was amended. If so, what would you include in the changelog / how would you phrase it? |
stuartmorgan-g
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.
I've rewritten the changelog; LGTM with that. Thanks for finding this!
|
@stuartmorgan Thanks a lot ❤️ |
cc @Hixie @stuartmorgan I explained here why
a breaking change was handled inproperly and this is whywe need to specify the dependency that is at least2.0.3.Before that version, the method head of
pushRouteNameToFrameworkwas different, which means that^2.0.0for the platform interface is a lie - any version before2.0.3is incompatible.Pre-launch Checklist
dart format. See plugin_tool format)[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.