Code review comment for lp:~niemeyer/juju-core/halve-tools-tarball-size

Revision history for this message
Roger Peppe (rogpeppe) wrote :

i see you've submitted, but a couple of comments for the record.

https://codereview.appspot.com/6579043/diff/4002/environs/tools.go
File environs/tools.go (right):

https://codereview.appspot.com/6579043/diff/4002/environs/tools.go#newcode442
environs/tools.go:442: {"go", "install",
"launchpad.net/juju-core/cmd/jujuc"},
better to have both commands installed in the same go command - that way
they share work.

similarly, you can pass two args to strip, although it probably doesn't
have the same benefits.

https://codereview.appspot.com/6579043/diff/4002/environs/tools.go#newcode452
environs/tools.go:452: return version.Binary{}, fmt.Errorf("build
failed: %v; %s", err, out)
it would be nice to know which command failed.

https://codereview.appspot.com/6579043/

« Back to merge proposal