Skip to content

Move Android doctor SDK check to 29 and Cirrus images to 30 #63517

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.

Sign up for GitHub

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

Jump to bottom
Merged
xster merged 6 commits into from
Aug 20, 2020
Merged

Move Android doctor SDK check to 29 and Cirrus images to 30 #63517

xster merged 6 commits into from
Aug 20, 2020

Conversation

@xster
Copy link
Member

@xster xster commented Aug 11, 2020 •
edited
Loading

Fixes #54382
Also move Cirrus to SDK 30 in the spirit of testing #56597

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) d: examples Sample code and demos c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Aug 11, 2020
@xster xster changed the title move compile/target sdk versions to 29 (#62799) Move Android doctor SDK check to 29 Aug 11, 2020
@xster xster requested a review from jmagman August 11, 2020 20:04
jmagman
jmagman reviewed Aug 11, 2020
View reviewed changes
packages/flutter_tools/lib/src/android/android_sdk.dart Outdated
Copy link
Member

@jmagman jmagman Aug 11, 2020

Choose a reason for hiding this comment

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

We shouldn't track this in two places. Can you update the create.dart usage (which populates the .iml template) to kAndroidSdkMinVersion and delete this?

Also, I wonder why the ide templates aren't using that, they hardcode the SDK version instead of using {{androidSdkVersion}}:
https://github.com/flutter/flutter/search?q=%22Android+API+25+Platform%22&unscoped_q=%22Android+API+25+Platform%22

Can you update those manually to 29? I created #63522 to change these to use the template context.

Copy link
Member Author

@xster xster Aug 11, 2020

Choose a reason for hiding this comment

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

sure. Done.

re: the ide template stuff, ya it's a bit unclean. I think it was a template to unpack IntelliJ projects for the Flutter SDK itself rather than a Flutter project so it doesn't have the same templates. Still worth cleaning up.

@jmagman
Copy link
Member

jmagman commented Aug 11, 2020 

[!] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
    • Android SDK at C:\Android
    X Flutter requires Android SDK 29 and the Android BuildTools 28.0.3
      To update using sdkmanager, run:
        "C:\Android\tools\bin\sdkmanager" "platforms;android-29" "build-tools;28.0.3"
      or visit https://flutter.dev/docs/get-started/install/windows#android-setup for detailed instructions.
    ! Some Android licenses not accepted.  To resolve this, run: flutter doctor --android-licenses

xster
xster commented Aug 11, 2020
View reviewed changes
.cirrus.yml Outdated
Copy link
Member Author

@xster xster Aug 11, 2020

Choose a reason for hiding this comment

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

@fkorotkov we're ready when you are :)

Copy link
Contributor

@fkorotkov fkorotkov Aug 12, 2020

Choose a reason for hiding this comment

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

what do you mean? 😅 seems the image is working for you 🤔

Copy link
Member Author

@xster xster Aug 12, 2020

Choose a reason for hiding this comment

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

ah ya LG. You were just mentioning on chat previously whether we needed 30. Awesome, thanks!

Copy link
Contributor

@fkorotkov fkorotkov Aug 12, 2020

Choose a reason for hiding this comment

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

oh, right. I was asking in the context of using 30 in the catalina-xcode-12.0-flutter. I'll upgrade it once this PR merged or it doesn't matter and I can bump it any time?

Copy link
Member Author

@xster xster Aug 12, 2020

Choose a reason for hiding this comment

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

You just meant creating a new catalina-xcode-12.0-flutter rather than moving all our current tests to catalina-xcode-12.0-flutter right? if so then ya, any time is good.

Copy link
Contributor

@fkorotkov fkorotkov Aug 12, 2020

Choose a reason for hiding this comment

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

So the question here is do you want Android 30 as part of this PR #63252? cc @jmagman

Copy link
Member Author

@xster xster Aug 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand your question, it's 2 separate things. This is Android 30 for Linux and Windows, that one is Xcode 12 for macOS. We can do them separately.

@xster xster changed the title Move Android doctor SDK check to 29 Move Android doctor SDK check to 29 and Cirrus images to 30 Aug 11, 2020
@fkorotkov fkorotkov mentioned this pull request Aug 14, 2020
Merged
@xster xster requested a review from jmagman August 20, 2020 05:32
jmagman
jmagman approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM!

@xster xster merged commit a2e5e07 into flutter:master Aug 20, 2020
@xster xster deleted the doctor-29 branch August 20, 2020 18:06
smadey pushed a commit to smadey/flutter that referenced this pull request Aug 27, 2020
@xster @smadey
Move Android doctor SDK check to 29 and Cirrus images to 30 (flutter#…
0520503 
…63517)
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@xster @mingwandroid
Move Android doctor SDK check to 29 and Cirrus images to 30 (flutter#…
149c7da 
…63517)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jmagman jmagman jmagman approved these changes

@christopherfujino christopherfujino Awaiting requested review from christopherfujino

+1 more reviewer

@fkorotkov fkorotkov fkorotkov left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. d: examples Sample code and demos tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update Android SDK from 28 to 29 in new plugins and project templates

4 participants

@xster @jmagman @fkorotkov @googlebot