Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support package_info_plus 4.x #1436

Closed
wants to merge 5 commits into from

Conversation

ThexXTURBOXx
Copy link
Contributor

📜 Description

This PR adds support for package_info_plus 4.x

💡 Motivation and Context

It fixes compatibility problems when using AGP 8.x by supporting a "fixed" version of package_info_plus.
There might be other libraries which do not support that yet, but they can probably be separately updated later.

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (bf4aed7) 90.79% compared to head (ebc6883) 90.79%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1436   +/-   ##
=======================================
  Coverage   90.79%   90.79%           
=======================================
  Files          61       61           
  Lines        2042     2042           
=======================================
  Hits         1854     1854           
  Misses        188      188           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marandaneto
Copy link
Contributor

@ThexXTURBOXx thanks, can you add a changelog entry? https://github.com/getsentry/sentry-dart/blob/main/CHANGELOG.md

@marandaneto
Copy link
Contributor

@ThexXTURBOXx also this:

/home/runner/work/sentry-dart/sentry-dart/flutter/example/android/app/src/main/AndroidManifest.xml Error:
uses-sdk:minSdkVersion 16 cannot be smaller than version 19 declared in library [:integration_test] /home/runner/work/sentry-dart/sentry-dart/flutter/example/build/integration_test/intermediates/merged_manifest/release/AndroidManifest.xml as the library might be using APIs not available in 16
Suggestion: use a compatible library with a minSdk of at most 16,
or increase this project's minSdk version to at least 19,
or use tools:overrideLibrary="dev.flutter.integration_test" to force usage (may lead to runtime failures)

Let's do the tools:overrideLibrary thingie.

@ThexXTURBOXx
Copy link
Contributor Author

ThexXTURBOXx commented May 11, 2023

I thought about this for a bit and it might be more advisable to add a breaking change instead and increment the min_sdk_version since 16 is not supported anymore anyways (because of the device_info_plus update).

Please let me know what you think.

@marandaneto
Copy link
Contributor

device_info_plus

Based on what I see, the class already checks SDK version for compatibility.
https://github.com/fluttercommunity/plus_plugins/blob/c8f7b6342a7c51eafafae95792775505d2b52ce9/packages/package_info_plus/package_info_plus/android/src/main/kotlin/dev/fluttercommunity/plus/packageinfo/PackageInfoPlugin.kt#L69

This is just to ensure that the SDK compiles, you can still force minAPi 19 in your final app, we compile our sample with the min. supported version as well


The AGP plugin checks if there's any class being used that is not supported < 16 and will fail the build, if that happens, then maybe increasing the version is the way to go.

@ThexXTURBOXx
Copy link
Contributor Author

Sorry, I of course meant package_info_plus.
I have added a changelog entry and updated the min_sdk_versions all over the place and it should build now. Let's see.

@ThexXTURBOXx
Copy link
Contributor Author

Since this is still not working I will close this in favor of #1440

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

Successfully merging this pull request may close these issues.

None yet

2 participants