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

Remove references to the V1 Android embedding #4160

Merged
merged 10 commits into from Jul 23, 2021

Conversation

blasten
Copy link

@blasten blasten commented Jul 15, 2021

Issue: flutter/flutter#86578

Remove references to the V1 Android embedding

@stuartmorgan
Copy link
Contributor

stuartmorgan commented Jul 16, 2021

Could you add an issue link with more context so that someone looking back at this change understands why this was done? (PRs with almost no context have been a significant problem for me as I've tried to investigate why various things in this repository are the way they are).

In this case, I thought in our previous discussion this was a multi-phase process where removing tests that we properly support the V1 embedding were going to wait for as long as we actually do support it, but it seems like those are being removed here.

@blasten
Copy link
Author

blasten commented Jul 16, 2021

Could you add an issue link with more context so that someone looking back at this change understands why this was done?

Done

. Is it actually important that we update those?

Ideally. Although, maybe not very urgent if their checks aren't running on CI

@stuartmorgan
Copy link
Contributor

. Is it actually important that we update those?

Ideally. Although, maybe not very urgent if their checks aren't running on CI

They are running on CI. If we need it for CI, but it's not critical for end users of the plugins, then we should update them but use NEXT instead of bumping the versions so that they don't publish new versions.

@blasten
Copy link
Author

blasten commented Jul 19, 2021

we should update them but use NEXT instead of bumping the versions so that they don't publish new versions.

Reverted the change.

@ditman
Copy link
Member

ditman commented Jul 20, 2021

we should update them but use NEXT instead of bumping the versions so that they don't publish new versions.

Reverted the change.

I think that we still should still publish the deprecated plugins, just to ensure that we don't have lingering documentation about the V1 embedding in the repo (so if the change touches the README.md of a plugin, it should be publipshed). (Not sure how "dangerous" is to have old school documentation lying around for some extra weeks/months on those plugins, though.)

Also @blasten, the latest change modified 3 plugins of which none are deprecated (I think?). Here's the list of deprecated plugins (or are we talking about a different deprecation?)

@blasten
Copy link
Author

blasten commented Jul 20, 2021

ah thanks. I just reverted all of them.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

I think that we still should still publish the deprecated plugins, just to ensure that we don't have lingering documentation about the V1 embedding in the repo

You mean on pub.dev? It's not in the repo whether we publish it or not.

The plugins are deprecated on pub.dev, so someone reading the instructions about integrating the plugin, on pub.dev, is already ignoring our recommendations. I'm not sure what problem we are solving by having updated instructions on starting to use plugins that we are telling people not to start using...

packages/android_alarm_manager/README.md Outdated Show resolved Hide resolved
@blasten
Copy link
Author

blasten commented Jul 21, 2021

PTAL @stuartmorgan

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@blasten blasten merged commit e843c03 into flutter:master Jul 23, 2021
@blasten blasten deleted the remove-v1-embedding branch July 23, 2021 18:04
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.