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

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Apr 4, 2019

I'd like to have this script reviewed and committed to our repo for our future reference.

This prepares the VM used in #8442

Once our Docker container is tested in GKE (after Kubernetes 1.14 is ready), I'll also upload Dockefiles and its related scripts here.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM

curl https://raw.githubusercontent.com/flutter/engine/master/ci/docker/build/engine_gclient `
-o c:/flutter/engine/.gclient

# Once all the environment variables above are loaded correctly, one may test
Copy link

Choose a reason for hiding this comment

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

It's better to tell the user what to do next at the end. In this script, probably add:

  1. This script finished successfully.
  2. A reboot of the terminal (or VM) is required for environment variables to be effective.
  3. After rebooting, run the following commands to test build the engine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think thi scomment is in reference to the state of the container rather than what the user should be doing at this point...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Tong and Dan for the comments! I think both of you are kind of correct: the steps that Tong mentioned are the next steps someone would do to test the correctness of the image; on the other hand, one can directly make an image without doing the test, so those next steps are optional.

I'll create another PR to add comments to clarify this. We can discuss there until it's clear what this script is doing and how someone should use it.

engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Apr 5, 2019
…e#8446) (#30538)

flutter/engine@8b74cba...697e541

git log 8b74cba..697e541 --no-merges --oneline
697e541 Add scripts that prepares our windows VM image (flutter/engine#8446)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (chinmaygarde@google.com), and stop
the roller if necessary.
@liyuqian liyuqian deleted the windows_vm_prepare branch April 5, 2019 19:46
RBogie pushed a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
Have this script reviewed and committed to our repo for our future reference.

This prepares the VM used in flutter#8442

Once our Docker container is tested in GKE (after Kubernetes 1.14 is ready), I'll also upload Dockefiles and its related scripts here.
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants