Skip to content

add a --deploy flag to build apk#3249

Merged
devoncarew merged 5 commits into
flutter:masterfrom
devoncarew:deploy_flag
Apr 12, 2016
Merged

add a --deploy flag to build apk#3249
devoncarew merged 5 commits into
flutter:masterfrom
devoncarew:deploy_flag

Conversation

@devoncarew

@devoncarew devoncarew commented Apr 11, 2016

Copy link
Copy Markdown
Contributor
  • add a --deploy (and a --develop) flag to flutter build apk. This doesn't do anything right now, but sets the stage for us to implement different build functionality for the two builds.
  • some minor cleanup (removing a method w/ one caller, and changing methods names to clarify when we're doing something android/apk specific)

@jason-simmons, @chinmaygarde I think the next step is to start building and uploading gen_snapshot into the darwin-x64 google storage bucket, and then using that binary when building the deploy APK, instead of the sky_snapshot binary. Does that sound right?

@eseidelGoogle

eseidelGoogle commented Apr 11, 2016

Copy link
Copy Markdown
Contributor

I recommend we fork Android Devices first since @chinmaygarde is OOO for a bit. I recommend just pointing deploy and develop at the exact same bits for now, we need some separate work to teach the flutter/engine/src/sky/tools/gn script about --deploy and teach the bots how to upload a separate engine flavor for --deploy. I suspect @jason-simmons may be the right person to do the GN work with @chinmaygarde out at the moment. @johnmccutchan can tell us what GN flags in Dart we should turn on in --deploy mode.

eseidelGoogle
eseidelGoogle reviewed Apr 11, 2016
View reviewed changes
Comment thread packages/flutter_tools/lib/src/commands/build_apk.dart Outdated
String flxPath,
ApkKeystoreInfo keystore
ApkKeystoreInfo keystore,
bool buildForDeploy: false

@eseidelGoogle eseidelGoogle Apr 11, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I probably would have used an enum here instead. Debug and Deploy are the only current options, but Profile is likely in the longer term?

@devoncarew devoncarew Apr 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sgtm

@devoncarew

devoncarew commented Apr 11, 2016

Copy link
Copy Markdown
Contributor Author

Updated to use an enum for the build variants instead of a bool param.

eseidelGoogle
eseidelGoogle reviewed Apr 11, 2016
View reviewed changes
Comment thread packages/flutter_tools/lib/src/commands/build_apk.dart Outdated
String flxPath,
ApkKeystoreInfo keystore
ApkKeystoreInfo keystore,
BuildVariant buildVariant: BuildVariant.develop

@eseidelGoogle eseidelGoogle Apr 11, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might want to make this a require parameter, or you could get strange behavior in the places where it's omitted by mistake?

@devoncarew devoncarew Apr 12, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do

@eseidelGoogle

eseidelGoogle commented Apr 11, 2016

Copy link
Copy Markdown
Contributor

lgtm

@eseidelGoogle eseidelGoogle mentioned this pull request Apr 12, 2016
Closed
@devoncarew devoncarew merged commit 8849cd6 into flutter:master Apr 12, 2016
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@devoncarew @eseidelGoogle