Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

collinjackson
Copy link
Contributor

@collinjackson collinjackson commented Jan 6, 2022

#4640 (reviewed by @stuartmorgan) updated plugin_plugin_interface to 2.1.0, soft-deprecating verifyToken and introducing its replacement, verify.

Now that 2.1.0 has been published, this PR updates all of the first-party plugins that used verifyToken to use verify and depend on ^2.1.0:

  • camera
  • file_selector
  • google_maps_flutter
  • image_picker
  • in_app_purchase
  • path_provider
  • quick_actions
  • url_launcher
  • video_player
  • webview_flutter

It is a follow-on fix for flutter/flutter#96178

Test plan

This PR has no new tests because it's not possible to test; there was no semantic change to the above plugins. They are not using const Object() as their token so a test cannot cause the new AssertionError to be thrown. I believe this means @Hixie has to sign off on it and I've posted in #hackers-ecosystem about that.

However, this PR does contain new tests in the sense that there are new tests in #4640 and this is a follow-on PR that could only be landed after that one did because of the plugin_platform_interface dependency. In some sense this repo is a giant test case and we're testing #4640 by creating lots of sample apps that depend on it.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • [too many plugins to list] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • [need approval] I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard
Copy link

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.

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.

@collinjackson collinjackson changed the title [url_launcher] Update first-party plugins to use the verify method in plugin_platform_interface [several] Update first-party plugins to use the verify method in plugin_platform_interface Jan 6, 2022
@collinjackson collinjackson force-pushed the update_plugin_platform_interface branch from d42b71b to 7abd84a Compare January 6, 2022 05:28
@collinjackson collinjackson changed the title [several] Update first-party plugins to use the verify method in plugin_platform_interface Update all plugins that use verifyToken to use verify instead Jan 6, 2022
@collinjackson collinjackson force-pushed the update_plugin_platform_interface branch from 963aec9 to 7abd84a Compare January 6, 2022 05:33
…0 versions, since 2.1.0 is forward-compatible.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG other than version issue

sdk: flutter
meta: ^1.3.0
plugin_platform_interface: ^2.0.0
plugin_platform_interface: '>=2.1.0 <4.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the final version of the platform interface chance doesn't require any breaking changes (the deprecation after this all lands is just a minor bump per semver and standard Dart package practice), these should all be ^2.1.0 instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what I had in a previous version of this PR, and I've reverted back to it.

…low 3.0.0 versions, since 2.1.0 is forward-compatible."

This reverts commit 7938460.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for doing all this cleanup!

@Hixie
Copy link
Contributor

Hixie commented Jan 6, 2022

test-exempt: code refactor with no semantic change

@stuartmorgan-g stuartmorgan-g added waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. last mile labels Jan 6, 2022
@fluttergithubbot
Copy link

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Jan 6, 2022
@godofredoc godofredoc changed the base branch from master to main January 6, 2022 22:09
@collinjackson collinjackson added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Jan 6, 2022
@fluttergithubbot fluttergithubbot merged commit 87adc2a into flutter:main Jan 7, 2022
@ditman
Copy link
Member

ditman commented Jan 7, 2022

Thanks for the cleanup @collinjackson! Long time no see!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants