LGTM with some tweaks and a suggestion
https://codereview.appspot.com/41660044/diff/1/cmd/cmd.go File cmd/cmd.go (right):
https://codereview.appspot.com/41660044/diff/1/cmd/cmd.go#newcode20 cmd/cmd.go:20: type rcPassthroughError struct { Why not use a type alias?
https://codereview.appspot.com/41660044/diff/1/cmd/cmd.go#newcode33 cmd/cmd.go:33: func NewRcPassthroughError(code int) error { Need doc here and/or the struct definition to say what it is for
https://codereview.appspot.com/41660044/diff/1/cmd/jujud/main.go File cmd/jujud/main.go (right):
https://codereview.appspot.com/41660044/diff/1/cmd/jujud/main.go#newcode118 cmd/jujud/main.go:118: code = cmd.Main(&RunCommand{}, cmd.DefaultContext(), args[1:]) Should we follow the pattern used for jujuc and jujud and have a jujurunMain method?
https://codereview.appspot.com/41660044/diff/1/cmd/jujud/run.go File cmd/jujud/run.go (right):
https://codereview.appspot.com/41660044/diff/1/cmd/jujud/run.go#newcode67 cmd/jujud/run.go:67: c.unit = names.UnitTag(c.unit) can we have a comment here explaining what is being done ie unit can be either a name or tag?
https://codereview.appspot.com/41660044/diff/1/environs/cloudinit/cloudinit.go File environs/cloudinit/cloudinit.go (right):
https://codereview.appspot.com/41660044/diff/1/environs/cloudinit/cloudinit.go#newcode242 environs/cloudinit/cloudinit.go:242: fmt.Sprintf("ln -s %s/tools/%s/jujud /usr/local/bin/juju-run", cfg.DataDir, machineTag), please use cfg.jujuTools()
https://codereview.appspot.com/41660044/
« Back to merge proposal
LGTM with some tweaks and a suggestion
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/cmd. go
File cmd/cmd.go (right):
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/cmd. go#newcode20
cmd/cmd.go:20: type rcPassthroughError struct {
Why not use a type alias?
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/cmd. go#newcode33 hError( code int) error {
cmd/cmd.go:33: func NewRcPassthroug
Need doc here and/or the struct definition to say what it is for
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/jujud/ main.go
File cmd/jujud/main.go (right):
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/jujud/ main.go# newcode118 main.go: 118: code = cmd.Main( &RunCommand{ }, ext(), args[1:])
cmd/jujud/
cmd.DefaultCont
Should we follow the pattern used for jujuc and jujud and have a
jujurunMain method?
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/jujud/ run.go
File cmd/jujud/run.go (right):
https:/ /codereview. appspot. com/41660044/ diff/1/ cmd/jujud/ run.go# newcode67 run.go: 67: c.unit = names.UnitTag( c.unit)
cmd/jujud/
can we have a comment here explaining what is being done ie unit can be
either a name or tag?
https:/ /codereview. appspot. com/41660044/ diff/1/ environs/ cloudinit/ cloudinit. go cloudinit/ cloudinit. go (right):
File environs/
https:/ /codereview. appspot. com/41660044/ diff/1/ environs/ cloudinit/ cloudinit. go#newcode242 cloudinit/ cloudinit. go:242: fmt.Sprintf("ln -s bin/juju- run", cfg.DataDir, machineTag),
environs/
%s/tools/%s/jujud /usr/local/
please use cfg.jujuTools()
https:/ /codereview. appspot. com/41660044/