-
Notifications
You must be signed in to change notification settings - Fork 28.9k
Try to make transitioning channels actually work. #14353
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
Conversation
@@ -93,7 +93,7 @@ class ChannelCommand extends FlutterCommand { | |||
|
|||
static Future<Null> _checkout(String branchName) async { | |||
final int result = await runCommandAndStreamOutput( | |||
<String>['git', 'checkout', branchName], | |||
<String>['git', 'checkout', '-b', branchName], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for users who're still on alpha and don't have a dev branch yet?
Scratch that - makes sense.
await doctor.diagnose(); | ||
code = await runCommandAndStreamOutput( | ||
<String>[ | ||
fs.path.join(Cache.flutterRoot, 'bin', 'flutter'), 'doctor', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if the working directory is already Cache.flutterRoot
, then you can omit Cache.flutterRoot
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we know what the working directory is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup - we're passing it explicitly in this invocation of runCommandAndStreamOutput()
} | ||
|
||
// Run a doctor check in case system requirements have changed. | ||
printStatus(''); | ||
printStatus('Running flutter doctor...'); | ||
await doctor.diagnose(); | ||
code = await runCommandAndStreamOutput( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need to be in a new process versus just invoking the doctor API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the doctor API is still the version from before we upgraded. It's really obvious in that the output after the upgrade still shows the old version number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this also fixed an issue whereby running the doctor API from the version before we upgraded could crash the tool when run against an upgraded tree -- https://gist.github.com/tvolkert/c2872310667f019969d9990fcade1ebb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Try to make transitioning channels actually work. * Update upgrade.dart
cc @tvolkert