This looks reasonable, but I'm not convinced by it living in jujud. The
jujud command is really an implementation detail and there's no
particular reason that it will be installed on a user's machine, which
is probably where charms are being written. Importing jujuc into juju
grows the binary by about 3%, but I don't think that's a big issue.
then append to it inside the next loop rather than assigning
to a known index? Then you won't need the "if cmds[i] != nil"
condition inside the print loop.
This looks reasonable, but I'm not convinced by it living in jujud. The
jujud command is really an implementation detail and there's no
particular reason that it will be installed on a user's machine, which
is probably where charms are being written. Importing jujuc into juju
grows the binary by about 3%, but I don't think that's a big issue.
I'd also like to see a test.
https:/ /codereview. appspot. com/11758043/ diff/1/ cmd/jujud/ helptool. go helptool. go (right):
File cmd/jujud/
https:/ /codereview. appspot. com/11758043/ diff/1/ cmd/jujud/ helptool. go#newcode20 helptool. go:20: func (_ dummyHookContext) UnitName() string {
cmd/jujud/
s/_ //
and below
https:/ /codereview. appspot. com/11758043/ diff/1/ cmd/jujud/ helptool. go#newcode79 helptool. go:79: cmds := make([]cmd.Command, len(names))
cmd/jujud/
How about:
cmds := make([]cmd.Command, 0, len(names))
then append to it inside the next loop rather than assigning
to a known index? Then you won't need the "if cmds[i] != nil"
condition inside the print loop.
https:/ /codereview. appspot. com/11758043/ diff/1/ cmd/jujud/ helptool. go#newcode89 helptool. go:89: const lineFormat = "%-*s %s\n"
cmd/jujud/
I don't think there's a benefit from separating the format from the
Fprintf here (it conceals it from govet)
https:/ /codereview. appspot. com/11758043/ diff/1/ cmd/jujud/ helptool. go#newcode90 helptool. go:90: const outputFormat = "%s"
cmd/jujud/
d (I'm quite surprised the compiler didn't complain about this being
unused actually)
https:/ /codereview. appspot. com/11758043/