[webview_flutter_wkwebview] Fix typo of a method name, setCookieForInsances #6102
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. |
|
Hi @kosukesaigusa, thanks for fixing this. Could you updated the CHANGELOG by following: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#updating-a-changelog-that-has-a-next. |
|
Thank you for your message, @bparrishMines. |
bparrishMines
left a comment
There was a problem hiding this comment.
I looks like you also need to pull in the latest changes to merge this.
| @@ -1,3 +1,7 @@ | |||
| ## NEXT | |||
|
|
|||
| * Fix typo in a method name, from `setCookieForInsances` to `setCookieForInstances`. | |||
There was a problem hiding this comment.
nit: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style
Should use Fixes instead of Fix.
|
@bparrishMines |
| @@ -1,6 +1,9 @@ | |||
| ## 2.9.1 | |||
| ## NEXT | |||
There was a problem hiding this comment.
When pulling in main it looks like you are erasing the 2.9.1 change. This should have been
## Next
* Fixes typo in a method name, from `setCookieForInsances` to `setCookieForInstances`.
## 2.9.1
* Fixes regression where the behavior for the `UIScrollView` insets were removed.
|
|
||
| /// Calls [setCookie] with the ids of the provided object instances. | ||
| Future<void> setCookieForInsances( | ||
| Future<void> setCookieForInstances( |
There was a problem hiding this comment.
This is technically public API, so we might need to do a breaking change for this.
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#breaking-changes
There was a problem hiding this comment.
The plugin doesn't expose these classes: https://github.com/flutter/plugins/blob/main/packages/webview_flutter/webview_flutter_wkwebview/lib/webview_flutter_wkwebview.dart.
However, Dart does still allow access to implementation files. But, there are lint warning about importing implementation files of a package and proper usage of the plugin will make accessing them difficult.
There was a problem hiding this comment.
Yeah, technically one can use those classes by importing webview_flutter_wkweview/lib/src/web_kit/web_kit_api.impls.dart, and those app will break after this change.
I'm not sure the policy of making "public" API change in these files.
There was a problem hiding this comment.
cc @stuartmorgan Is there documented policy on changing files in lib/src in a plugin?
In Dart's documentation on implementation files, it looks to be implied that changing files in lib/src are not considered a breaking change. As a user shouldn't be importing them: https://dart.dev/tools/pub/package-layout#implementation-files
There was a problem hiding this comment.
Anything we're not exporting is fair game to change.
|
Is the only thing remaining here the CHANGELOG issue? @kosukesaigusa can you rebase/merge onto top of tree and make sure your pubpsec and CHANGELOG version matches? |
|
Friendly ping, @kosukesaigusa, this is so close to being mergeable! 🙂 |
Just fixed a typo of a method name, from
setCookieForInsancestosetCookieForInstancesNo version change: Only updates internal method name.
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.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.