fix(package_info_plus): fixed deprecation warning & updated dependencies by juliansteenbakker · Pull Request #442 · fluttercommunity/plus_plugins

juliansteenbakker · 2021-09-02T13:43:08Z

Description

This PR removes the deprecated embedding V1 from package_info_plus. Besides that it updates the dependencies and fixes a deprecation issue with regards to getBuildSignature method.

Related Issues

Probably fixes #301

Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (flutter analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

mhadaily · 2021-10-01T18:37:58Z

I just updated main branch can you please update with main. and then let's merge your branch.

juliansteenbakker · 2021-10-01T19:35:00Z

I just updated main branch can you please update with main. and then let's merge your branch.

I will but before merging, should we remove the deprecated registerwith function, or just leave it in and annotate so it hides the warning? Because whatever we chose, we should apply it to all other packages aswell.

mhadaily · 2021-10-01T19:37:58Z

Well, I'd say if we have a solution to replace it with a new API, let's do that, that much better, however, if we don't lets at least annotate it so later we will remove depreciated API and replace them with a new one.

I prefer to implement a new API if possible.

juliansteenbakker · 2021-10-01T20:13:52Z

Currently v1 embedding with registerWith is only kept for backwards compatibility. However it seems that it isn't included in new releases according to official Flutter documentation. So i'd say it's safe to remove it.

nohli · 2021-10-01T20:14:12Z

@juliansteenbakker
It is my understanding that it is good to keep the registerWith() method to remain compatible:

Also, note that the plugin should still contain the static registerWith() method to remain compatible with apps that don’t use the v2 Android embedding. (See Upgrading pre 1.12 Android projects for details.) The easiest thing to do (if possible) is move the logic from registerWith() into a private method that both registerWith() and onAttachedToEngine() can call. Either registerWith() or onAttachedToEngine() will be called, not both.

https://flutter.dev/docs/development/packages-and-plugins/plugin-api-migration#upgrade-steps

@mhadaily
I added PRs that suppress the build warnings for all remaining packages. Didn't see that for package_info_plus there is already this one. I can close mine probably?

juliansteenbakker · 2021-10-01T20:31:02Z

@nohli It's more a question of choice i think. The flutter team encourages everyone to upgrade to V2, and right now only 20% of the projects use V1. You can keep providing support for V1 but where are you going to draw the line? Besides that, is it even possible for Flutter applications that only support V1 embedding to also support null-safety, like all packages here does?

juliansteenbakker · 2021-10-01T20:32:42Z

@nohli You can close your pr for this package. I'll update this to leave the registerWith in it if thats what we're deciding.

… and added changelog entry

mhadaily · 2021-10-02T10:34:36Z

@nohli @juliansteenbakker let's discuss there and make a decision, I don't think it's very critical to support V1. what we can do we can remove this and release a new Major version for packages so if anyone likes to use older version so still, they can but we will not support it?

what do you think?

question for you @nohli if you think this PR is covering your PRs, let's close yours and focus on this one if you agree?

mhadaily · 2021-10-02T10:35:05Z

@juliansteenbakker build failed, I think you need to rebase with main

nohli · 2021-10-02T10:37:00Z

Already closed.

So you want me to close all the other PRs, and open a new one, removing the registerWith method, for all other packages?

juliansteenbakker · 2021-10-04T06:47:32Z

As discussed in #461 i've deleted the v1 embedding once again and bumped the version number.

Currently blocked by #489

mhadaily · 2021-10-05T01:42:25Z

@juliansteenbakker PR is merged.

btw, since we have now this PR #495 do you think your PR will be still valid?

# Conflicts: # packages/sensors_plus/sensors_plus/test/sensors_test.dart # packages/sensors_plus/sensors_plus_platform_interface/test/sensors_plus_platform_interface_test.dart # packages/share_plus/share_plus_platform_interface/test/share_plus_platform_interface_test.dart

juliansteenbakker · 2021-10-05T06:37:43Z

btw, since we have now this PR #495 do you think your PR will be still valid?

Yes because that PR doesn't contain changes for package_info_plus. Also, this PR solves other deprecation problems besides embedding V1.

juliansteenbakker · 2021-10-05T07:50:44Z

@mhadaily this is ready to be merged

nohli · 2021-10-05T10:41:32Z

@juliansteenbakker could you also use 30 for compileSdkVersion and targetSdkVersion?
31 is Android 12 which is still in beta.

vbuberen · 2021-10-05T11:09:43Z

31 is Android 12 which is still in beta.

It is not in beta since yesterday, but I would be careful with pushing to 31, because it has some behavior changes. For example, android_alarm_manager should be upgraded carefully, considering this change https://developer.android.com/about/versions/12/behavior-changes-12#exact-alarm-permission
Haven't checked if other packages would need the same though

juliansteenbakker · 2021-10-05T11:14:42Z

@nohli Android 12 is not in beta anymore. Besides, when a new number is available in compileSdkVersion, it means that the API is stable. Otherwise it would be marked as such: compileSdkVersion 'android-R'. compileSdkVersion 31 has been stable since 14 July.

juliansteenbakker · 2021-10-05T11:16:31Z

31 is Android 12 which is still in beta.

It is not in beta since yesterday, but I would be careful with pushing to 31, because it has some behavior changes. For example, android_alarm_manager should be upgraded carefully, considering this change https://developer.android.com/about/versions/12/behavior-changes-12#exact-alarm-permission Haven't checked if other packages would need the same though

We should migrate and check every package individually. That is why this PR is only about package_info_plus.

vbuberen · 2021-10-05T11:18:00Z

We should migrate and check every package individually

Yes, agree here. My message was mostly to @nohli since he has one big PR with changes to multiple packages, so I wanted to bring attention to that moment about Android 12.

miquelbeltran

LGTM!

juliansteenbakker added 9 commits September 2, 2021 15:16

imp: updated dependencies & gradle

be03611

imp: fixed library importing not working correctly

81d3e30

imp: removed deprecated embedding v1

afa7e92

imp: fixed packageManager deprecation & other minor improvements

1a2a88e

test: fix integration test

8b76998

imp: update gradle

c8bdda8

style: format

343e5e4

imp: upgrade test and dependencies

31918a7

imp: upgrade gradle and android compile version

90454f9

This was referenced Oct 1, 2021

Who is merging open PR's? #475

Closed

ci: upgrade flutter from 2.2.1 to 2.5.2 (blocked by #491) #476

Closed

migrate integration_test to flutter sdk #466

Merged

mhadaily self-requested a review October 1, 2021 19:21

mhadaily assigned juliansteenbakker Oct 1, 2021

mhadaily added the Hacktoberfest Issues taking part in Hacktoberfest label Oct 1, 2021

nohli mentioned this pull request Oct 1, 2021

fix(device_info_plus): suppress Android build deprecation warnings #461

Closed

9 tasks

nohli mentioned this pull request Oct 1, 2021

fix(package_info_plus): Suppress Android build deprecation warnings #480

Closed

9 tasks

juliansteenbakker added 3 commits October 1, 2021 22:57

refactor: reverted embedding v1 function + annotation, bumped version…

a738e1a

… and added changelog entry

bug: added registry import

c371e9a

Merge branch 'main' into deprecation

5b33f18

juliansteenbakker added 4 commits October 3, 2021 20:51

Merge branch 'main' into deprecation

03b3575

imp: fix test

a9c2669

test: removed unused test import

98b2705

test: downgrade because of version mismatch

ca3d1bf

juliansteenbakker mentioned this pull request Oct 3, 2021

Update Melos and CD/CI #489

Merged

9 tasks

juliansteenbakker added 2 commits October 4, 2021 08:43

imp: removed v1 embedding as discussed in fluttercommunity#461

7c1db24

bump version to 1.3.0 and updated changelog

48683c4

juliansteenbakker changed the title ~~fix(package_info_plus): fixed deprecation warning & updated dependencies~~ fix(package_info_plus): fixed deprecation warning & updated dependencies (blocked by #489) Oct 4, 2021

juliansteenbakker added 2 commits October 4, 2021 11:52

imp: removed unused import

f1ea833

Merge branch 'fluttercommunity:main' into deprecation

2c0cfb5

juliansteenbakker changed the title ~~fix(package_info_plus): fixed deprecation warning & updated dependencies (blocked by #489)~~ fix(package_info_plus): fixed deprecation warning & updated dependencies Oct 5, 2021

juliansteenbakker added 3 commits October 5, 2021 08:44

style: reformat file

d82c81b

style: reformat file

3e8465c

style: reformat file

61879c6

juliansteenbakker mentioned this pull request Oct 5, 2021

[all] Upgrade to latest gradle versions #505

Closed

miquelbeltran approved these changes Oct 6, 2021

View reviewed changes

miquelbeltran added the hacktoberfest-accepted label Oct 6, 2021

miquelbeltran merged commit 70ca955 into fluttercommunity:main Oct 6, 2021

mhadaily mentioned this pull request Nov 19, 2021

Announcing Hacktoberfest contributors and winners #606

Closed

github-actions Bot locked as resolved and limited conversation to collaborators Feb 4, 2025

Uh oh!

Conversation

juliansteenbakker commented Sep 2, 2021

Description

Related Issues

Checklist

Breaking Change

Uh oh!

mhadaily commented Oct 1, 2021

Uh oh!

juliansteenbakker commented Oct 1, 2021

Uh oh!

mhadaily commented Oct 1, 2021

Uh oh!

juliansteenbakker commented Oct 1, 2021

Uh oh!

nohli commented Oct 1, 2021

Uh oh!

juliansteenbakker commented Oct 1, 2021

Uh oh!

juliansteenbakker commented Oct 1, 2021

Uh oh!

mhadaily commented Oct 2, 2021

Uh oh!

mhadaily commented Oct 2, 2021

Uh oh!

nohli commented Oct 2, 2021

Uh oh!

juliansteenbakker commented Oct 4, 2021

Uh oh!

mhadaily commented Oct 5, 2021

Uh oh!

juliansteenbakker commented Oct 5, 2021

Uh oh!

juliansteenbakker commented Oct 5, 2021

Uh oh!

nohli commented Oct 5, 2021

Uh oh!

vbuberen commented Oct 5, 2021

Uh oh!

juliansteenbakker commented Oct 5, 2021

Uh oh!

juliansteenbakker commented Oct 5, 2021

Uh oh!

vbuberen commented Oct 5, 2021

Uh oh!

miquelbeltran left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants