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

package upgrades #842

Merged
merged 4 commits into from
Jul 30, 2024
Merged

package upgrades #842

merged 4 commits into from
Jul 30, 2024

Conversation

vaishnavi-2301
Copy link
Contributor

Update compileSdkVersion and dependencies.

@vaishnavi-2301
Copy link
Contributor Author

@diegotori please let me know if there's a requirement of updating the files in android folder according to the new migration guide https://docs.flutter.dev/release/breaking-changes/flutter-gradle-plugin-apply

will be happy to contribute!!

@diegotori
Copy link
Collaborator

@vaishnavi-2301 please address the lint issues.

Also, since this library is merely a UI layer over video_player, the only thing that needs migrating is the Android example app.

…n material_controls.dart and material_desktop_controls.dart
@vaishnavi-2301
Copy link
Contributor Author

@diegotori Hey, addressed the lint issues. kindly review and let me know if there's anything required.

Copy link
Collaborator

@diegotori diegotori left a comment

Choose a reason for hiding this comment

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

Please ensure that the Android example app is closely configured based on this example from wakelock_plus.

The reason being is that I recently had to shake some bugs when actually building the migrated Android app. The example that I linked you to should be the gold standard in terms of that.

Mainly, it involves building the app against Java 17, but please cross-check your changes against that example.

Thanks in advance.

@vaishnavi-2301
Copy link
Contributor Author

vaishnavi-2301 commented Jul 17, 2024

Thanks for the reference....have updated the files according to it. appologies for the delay.

@vaishnavi-2301 vaishnavi-2301 requested a review from diegotori July 29, 2024 12:46
minSdkVersion 21
targetSdkVersion 33
applicationId "com.example.example"
minSdk 24
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vaishnavi-2301 Is there any reason why minSdk on Android needs to be set at 24?

Unless you cite where it needs to be set that high, please set it to 21, which is the bare minimum that Flutter now supports. Thanks.

Copy link
Contributor Author

@vaishnavi-2301 vaishnavi-2301 Jul 30, 2024

Choose a reason for hiding this comment

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

Okay will change it to the 21

@vaishnavi-2301 vaishnavi-2301 requested a review from diegotori July 30, 2024 09:25
Copy link
Collaborator

@diegotori diegotori left a comment

Choose a reason for hiding this comment

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

LGTM.

@diegotori diegotori merged commit 16a0659 into fluttercommunity:master Jul 30, 2024
2 checks passed
@diegotori
Copy link
Collaborator

@vaishnavi-2301 added in version 1.8.2.

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.

3 participants