Skip to content

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

Closed
nohli wants to merge 4 commits into
fluttercommunity:mainfrom
nohli:suppress-android-deprecation-warnings
Closed

fix(device_info_plus): suppress Android build deprecation warnings#461
nohli wants to merge 4 commits into
fluttercommunity:mainfrom
nohli:suppress-android-deprecation-warnings

Conversation

@nohli

@nohli nohli commented Sep 18, 2021

Copy link
Copy Markdown
Member

Description

The plugin is updated to use Android v2 embedding, while keeping the registerWith() method for keeping backwards compatibility.
Since it's deprecated, there are warnings on every Android build.
This PR suppresses the warnings.

Related Issues

/opt/homebrew/Caskroom/flutter/2.5.0/flutter/.pub-cache/hosted/pub.dartlang.org/device_info_plus-2.1.0/android/src/main/java/dev/fluttercommunity/plus/device_info/DeviceInfoPlusPlugin.java:11: warning: [deprecation] Registrar in PluginRegistry has been deprecated
import io.flutter.plugin.common.PluginRegistry.Registrar;
                                              ^
/opt/homebrew/Caskroom/flutter/2.5.0/flutter/.pub-cache/hosted/pub.dartlang.org/device_info_plus-2.1.0/android/src/main/java/dev/fluttercommunity/plus/device_info/DeviceInfoPlusPlugin.java:19: warning: [deprecation] Registrar in PluginRegistry has been deprecated
  public static void registerWith(Registrar registrar) {
                                  ^
2 warnings

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • 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.

@nohli nohli changed the title fix(device_info_plus): suppress Android deprecation build warnings fix(device_info_plus): suppress Android build deprecation warnings Sep 18, 2021
@mhadaily mhadaily added the Hacktoberfest Issues taking part in Hacktoberfest label Oct 1, 2021

@mhadaily mhadaily left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM,

@nohli I wish you can send similar PR for other packages too. that would be awesome.

@nohli

nohli commented Oct 1, 2021

Copy link
Copy Markdown
Member Author

LGTM,

@nohli I wish you can send similar PR for other packages too. that would be awesome.

Ok, will do! 🙂

Thanks for adding hacktoberfest-accepted!

@nohli

nohli commented Oct 1, 2021

Copy link
Copy Markdown
Member Author

@mhadaily I opened PRs for the six other packages that use the deprecated method without suppressing warnings.

Would be cool if you also added the hacktoberfest-accepted tag.

I also updated the version and changelog to save you time, hope that's ok.

@juliansteenbakker

Copy link
Copy Markdown
Contributor

@nohli wouldn't it be better to remove the whole registerWith function, since new Flutter plugins also don't ship with this function anymore. See https://flutter.dev/docs/release/breaking-changes/android-v1-embedding-create-deprecation for more info.

@nohli

nohli commented Oct 1, 2021

Copy link
Copy Markdown
Member Author

@juliansteenbakker
Did you see my comment and the link in #442 (comment)?

Flutter advises to keep this method. Idk how old the linked docs are.

If new packages don't use the method any more, in my opinion it's okay because it's no breaking change for old users (that don't exist).
Removing it from a package that has been around for some time would be a breaking change, right?

@juliansteenbakker

Copy link
Copy Markdown
Contributor

@juliansteenbakker Did you see my comment and the link in #442 (comment)?

Flutter advises to keep this method. Idk how old the linked docs are.

If new packages don't use the method any more, in my opinion it's okay because it's no breaking change for old users (that don't exist). Removing it from a package that has been around for some time would be a breaking change, right?

I think you are right. I checked all official Google plugins and they all contain registerWith function. I'll change my PR accordingly (#442)

Just one more question, should the version of this package not be 2.2.1 instead of 2.3.0? It doesn't seem like that big of a change.

@mhadaily

mhadaily commented Oct 2, 2021

Copy link
Copy Markdown
Member

@nohli this is great, but can we focus on one PR for all of the package related to this, I commented on your PRs.

thanks

@mhadaily

mhadaily commented Oct 3, 2021

Copy link
Copy Markdown
Member

@nohli let me know when you can close all other PR and merge them all into one PR

thanks

@nohli

nohli commented Oct 3, 2021

Copy link
Copy Markdown
Member Author

@mhadaily have we decided on how to handle the deprecations?
Before opening another PR, I would like to clearly know the requirements.

The registerWith method takes Registrar which is deprecated.
Options:

  • Removing the method altogether and stop supporting Android v1 embedding
    -> breaking change with major version upgrade?
  • Keeping the method and suppressing the deprecation warning

And do you want me to also close this PR and open a new one?

@mhadaily

mhadaily commented Oct 3, 2021

Copy link
Copy Markdown
Member

Let us remove Android v1 embedding support and release a new major version.
Let us be fully compatible with new APIs as most Flutter developers also use v2 embedding.

If there are still apps that use v1, they can use an older version. I don't see any big concern about removing V1 support as long as we release a major version.

I'd recommend you also take a look at FlutterFire to see if they follow.

thoughts?

@nohli

nohli commented Oct 3, 2021

Copy link
Copy Markdown
Member Author

Okay, will do 😊

Your last sentence means to also open a similar PR over there?

Do you want me to also close this PR?

@mhadaily

mhadaily commented Oct 3, 2021

Copy link
Copy Markdown
Member

If you are comfortable with this PR, please continue on this one and please close all others. I just like to see one PR for this issue to be honest. I'll leave it to you whether you open a new one or continue on this one, both works for me.

@mhadaily mhadaily left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as we discussed

@nohli

nohli commented Oct 4, 2021

Copy link
Copy Markdown
Member Author

Closing because of #495

@nohli nohli closed this Oct 4, 2021
miquelbeltran pushed a commit that referenced this pull request Oct 6, 2021
…ies (#442)

* imp: updated dependencies & gradle

* imp: fixed library importing not working correctly

* imp: removed deprecated embedding v1

* imp: fixed packageManager deprecation & other minor improvements

* test: fix integration test

* imp: update gradle

* style: format

* imp: upgrade test and dependencies

* imp: upgrade gradle and android compile version

* refactor: reverted embedding v1 function + annotation, bumped version and added changelog entry

* bug: added registry import

* imp: bump version

* bug: removed double integration test declaration

* imp: fix test

* test: removed unused test import

* test: downgrade because of version mismatch

* imp: removed v1 embedding as discussed in #461

* bump version to 1.3.0 and updated changelog

* imp: removed unused import

* style: reformat file

* style: reformat file

* style: reformat file
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Hacktoberfest Issues taking part in Hacktoberfest hacktoberfest-accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants