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

Bump plugin Android compileSdkVersions to 31 #4502

Merged
merged 52 commits into from
Nov 19, 2021
Merged

Bump plugin Android compileSdkVersions to 31 #4502

merged 52 commits into from
Nov 19, 2021

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented Nov 11, 2021 •
edited
Loading

Bumps compileSdkVersion of the following plugin example apps to 31: android_alarm_manager, android_intent, battery, connectivity, device_info, espresso, flutter_plugin_android_lifecycle, google_maps_flutter, google_sign_in, image_picker, in_app_purchase, local_auth, package_info, path_provider, quick_actions, sensors, share, share_preferences, url_launcher, video_player, webview_flutter, wifi_info_flutter.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Nov 11, 2021
@Hixie
Copy link
Contributor

Hixie commented Nov 17, 2021

test-exempt: already tested per @stuartmorgan's comment above

stuartmorgan-g
stuartmorgan-g approved these changes Nov 17, 2021
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with nits. Sorry there ended up being a lot of review churn in the minor details here.

gaaclarke
gaaclarke reviewed Nov 17, 2021
View reviewed changes
...ick_actions/android/src/main/java/io/flutter/plugins/quickactions/MethodCallHandlerImpl.java
@Override
public void run() {
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
shortcutManager.setDynamicShortcuts(shortcuts);
Copy link
Member

Choose a reason for hiding this comment

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

Right now you are calling result after you've dispatched the message to the background thread. You should call the result after the background task has executed (calling result from the background thread should be thread-safe now, no need to dispatch back to the main thread).

Copy link
Contributor

Choose a reason for hiding this comment

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

calling result from the background thread should be thread-safe now

Does "now" include stable? Plugins support at least back to last stable (often more, but currently for this plugin it's 2.5)

Copy link
Member

Choose a reason for hiding this comment

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

No, it doesn't I corrected myself over chat. We can't call the result directly until background platform channels is on stable.

@Piinks Piinks mentioned this pull request Nov 17, 2021
Merged
8 tasks
@camsim99 camsim99 requested a review from blasten November 18, 2021 00:28
blasten
blasten approved these changes Nov 18, 2021
View reviewed changes
Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM

stuartmorgan-g
stuartmorgan-g suggested changes Nov 18, 2021
View reviewed changes
...ick_actions/android/src/main/java/io/flutter/plugins/quickactions/MethodCallHandlerImpl.java Outdated
result.error(
"quick_action_setshortcutitems_failure",
"Exception thrown when setting dynamic shortcuts",
null);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is on the wrong thread; any interaction with result needs to be (on current stable) on the main thread.

stuartmorgan-g
stuartmorgan-g approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

One last nit, otherwise LGTM

@camsim99 camsim99 added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Nov 19, 2021
@fluttergithubbot fluttergithubbot merged commit 943b604 into flutter:master Nov 19, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 19, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 19, 2021
@engine-flutter-autoroll
943b604 Bump plugin Android compileSdkVersions to 31 (flutter/plugins…
dbac9ee 
…#4502)
@gaaclarke gaaclarke mentioned this pull request Dec 1, 2021
Closed
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
@camsim99 @amantoux
Bump plugin Android compileSdkVersions to 31 (flutter#4502)
2515f13
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
@camsim99 @KyleFin
Bump plugin Android compileSdkVersions to 31 (flutter#4502)
ef3624b
@bparrishMines bparrishMines mentioned this pull request Aug 31, 2022
Merged
@stuartmorgan-g stuartmorgan-g mentioned this pull request Oct 16, 2023
Merged
11 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

@LHLL LHLL Awaiting requested review from LHLL

4 more reviewers

@gaaclarke gaaclarke gaaclarke left review comments

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@blasten blasten blasten approved these changes

@Piinks Piinks Piinks approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes needs tests p: android_alarm_manager p: android_intent p: battery Edits files for a battery plugin p: connectivity p: device_info p: espresso p: flutter_plugin_android_lifecycle p: google_maps_flutter p: google_sign_in p: image_picker p: in_app_purchase p: local_auth p: package_info p: path_provider p: quick_actions p: sensors p: share p: shared_preferences p: url_launcher p: video_player p: webview_flutter Edits files for a webview_flutter plugin p: wifi_info_flutter platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@camsim99 @stuartmorgan-g @blasten @Hixie @Piinks @gaaclarke @fluttergithubbot