Code review comment for lp:~thumper/juju-core/find-jujud

Revision history for this message
William Reade (fwereade) wrote :

https://codereview.appspot.com/11326043/diff/1/environs/tools/build.go
File environs/tools/build.go (right):

https://codereview.appspot.com/11326043/diff/1/environs/tools/build.go#newcode212
environs/tools/build.go:212: if err := copyExistingJujud(dir); err !=
nil {
Actually, this has percolated a bit and I'm not so happy. This is quite
a big and magical behaviour change... I have on a few occasions
needed/wanted to upload tools that *don't* match the juju binary I'm
using, and this kinda breaks that. Can still be worked around; and
should probably only affect developers; but I'm starting to think that
it's wrong to piggyback on the upload-tools mechanism for this use case.

I should be around tonight to talk about it a little... my big concern
is that we're changing behavior that *will* affect all the users who are
using upload-tools. Maybe I can be convinced that it's only a little,
and maybe even a beneficial, change; but let's discuss tonight.

https://codereview.appspot.com/11326043/

« Back to merge proposal