Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Write to Stderr directly for error output. #51
Conversation
anastasiamac
approved these changes
Apr 13, 2017
Looks really neat \o/
Just couple of comments but nothing blocking, I think :D
Thank you!
| + | ||
| +// NewSeqPrompter returns a prompter that can be used to check a sequence of | ||
| +// IO interactions. Expected input from the user is marked with the | ||
| +// given user input marker (for example a distinctive unicode character |
| + p.pendingError = nil | ||
| + return 0, err | ||
| + } | ||
| + panic("unreachable") |
anastasiamac
Apr 13, 2017
Member
Is this really necessary here? It looks to me like you've covered all the possibilities - when would you expect to panic here now?
If you strongly believe in its necessity, I think we'd want to unit test that we can reach it ;D
howbazaar
Apr 13, 2017
Owner
These files were just copied from the juju/juju/cmd/cmdtesting, and the only thing I changed was the licence header.
| + jc "github.com/juju/testing/checkers" | ||
| + gc "gopkg.in/check.v1" | ||
| + | ||
| + "github.com/juju/cmd/cmdtesting" |
| gc "gopkg.in/check.v1" | ||
| + | ||
| + "github.com/juju/cmd" |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-cmd |
jujubot
merged commit e47739a
into
juju:master
Apr 13, 2017
howbazaar
deleted the
howbazaar:logging-fix
branch
Apr 13, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
howbazaar commentedApr 12, 2017
Using the logger for error output was a bad call.
This branch also brings in the non-juju specific extra methods used for testing commands.