LGTM. A few suggestions, but feel free to submit:
https://codereview.appspot.com/6249075/diff/1/cmd/juju/cmd_test.go File cmd/juju/cmd_test.go (right):
https://codereview.appspot.com/6249075/diff/1/cmd/juju/cmd_test.go#newcode193 cmd/juju/cmd_test.go:193: os.Setenv("JUJU_REPOSITORY", "/path/to/repo") The test should preserve the environment as it found before the test started. E.g.:
defer os.Setenv("JUJU_REPOSITORY", os.Getenv("JUJU_REPOSITORY"))
The list of tests below look very appropriate for a table of tests as well. Something along the lines of:
var deployTests = []struct{ args []string cmd *DeployCommand }{ { {"charm-name", "service-name"}, &DeployCommand{ ... }, }, { ... }, }
https://codereview.appspot.com/6249075/diff/1/cmd/juju/deploy.go File cmd/juju/deploy.go (right):
https://codereview.appspot.com/6249075/diff/1/cmd/juju/deploy.go#newcode22 cmd/juju/deploy.go:22: "deploy", "<charm-name> [<service-name>]", "deploy a new service", "", We can avoid the dashes in these cases. <charm name> and <service name> read quite nicely.
Also, can you please document the fact that <service name> defaults to <charm name> if not provided?
https://codereview.appspot.com/6249075/
« Back to merge proposal
LGTM. A few suggestions, but feel free to submit:
https:/ /codereview. appspot. com/6249075/ diff/1/ cmd/juju/ cmd_test. go cmd_test. go (right):
File cmd/juju/
https:/ /codereview. appspot. com/6249075/ diff/1/ cmd/juju/ cmd_test. go#newcode193 cmd_test. go:193: os.Setenv( "JUJU_REPOSITOR Y", "/path/to/repo")
cmd/juju/
The test should preserve the environment as it found before the test
started. E.g.:
defer os.Setenv( "JUJU_REPOSITOR Y", os.Getenv( "JUJU_REPOSITOR Y"))
The list of tests below look very appropriate for a table of tests as
well. Something along the lines of:
var deployTests = []struct{
{ "charm- name", "service-name"},
& DeployCommand{
...
args []string
cmd *DeployCommand
}{
{
},
}, {
...
},
}
https:/ /codereview. appspot. com/6249075/ diff/1/ cmd/juju/ deploy. go
File cmd/juju/deploy.go (right):
https:/ /codereview. appspot. com/6249075/ diff/1/ cmd/juju/ deploy. go#newcode22 deploy. go:22: "deploy", "<charm-name> [<service-name>]",
cmd/juju/
"deploy a new service", "",
We can avoid the dashes in these cases. <charm name> and <service name>
read quite nicely.
Also, can you please document the fact that <service name> defaults to
<charm name> if not provided?
https:/ /codereview. appspot. com/6249075/