Skip to content
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

Upgrade the CLI to use Go 1.12. #1443

Merged
merged 1 commit into from
May 9, 2019
Merged

Conversation

armandgrillet
Copy link
Contributor

This does not change our supported operating systems, Go 1.12 being the
last release supporting macOS 10.10 Yosemite.

@armandgrillet
Copy link
Contributor Author

@mesosphere-ci run integration tests

@armandgrillet
Copy link
Contributor Author

Waiting for docker-library/golang#264 to be merged.

@bamarni
Copy link
Contributor

bamarni commented Feb 26, 2019

The new ProcessState.ExitCode method returns the process's exit code.

We should be able to simplify this by using exitErr.ExitCode():

dcos-cli/pkg/cmd/dcos.go

Lines 234 to 244 in b4a7d91

// When the plugin command exits with a non-zero code, the main CLI process
// exit code should be the same.
//
// see https://jira.mesosphere.com/browse/DCOS_OSS-4399
if exiterr, ok := err.(*exec.ExitError); ok {
if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
// TODO: have a more generic error handling system
// where errors can be associated with an exit code.
os.Exit(status.ExitStatus())
}
}

Wanna give it a try in this PR?

@armandgrillet
Copy link
Contributor Author

@mesosphere-ci run integration tests

@armandgrillet
Copy link
Contributor Author

@mesosphere-ci run integration tests

pkg/cmd/dcos.go Outdated Show resolved Hide resolved
pkg/cmd/dcos.go Outdated Show resolved Hide resolved
pkg/cmd/dcos.go Outdated Show resolved Hide resolved
bamarni
bamarni previously approved these changes Feb 27, 2019
Copy link
Contributor

@bamarni bamarni left a comment

Choose a reason for hiding this comment

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

LGTM, we will have to wait for Appveyor to support Go 1.12. If they do not do it soon we'll investigate on alternative.

rgo3
rgo3 previously approved these changes Feb 28, 2019
@armandgrillet
Copy link
Contributor Author

For Appveyor: appveyor/ci#2875

@armandgrillet
Copy link
Contributor Author

@mesosphere-ci run integration tests

@armandgrillet
Copy link
Contributor Author

@mesosphere-ci run integration tests

@armandgrillet armandgrillet requested review from bamarni and rgo3 and removed request for bamarni May 9, 2019 08:13
@bamarni bamarni merged commit 54c7745 into dcos:master May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants