Code review comment for lp://staging/~fwereade/juju-core/environs-tools-final-replacement
Revision history for this message
| Roger Peppe (rogpeppe) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
7dafbc3
demo site
(Get the code!)
https:/
File cmd/juju/
https:/
cmd/juju/
public tools would be ignored")
On 2013/04/16 08:43:40, fwereade wrote:
> On 2013/04/15 13:48:21, rog wrote:
> > i think this should be just a warning, if anything.
> > just because we have private tools present in the current
environment doesn't
> > mean we don't want to make tools available to others in the public
bucket.
> Honestly, I question the value of --public regardless. If I were
setting it up
> I'd have a shared-tools environ that I never actually used other than
to sync
> tools into, and use its control-bucket as my other environs'
public-bucket.
> Given the current fallback rules, a public sync-tools into an env with
private
> tools will fail to perform the purpose of sync-tools -- make them
*available* to
> the target env -- and so I think it's actually better to fail before
performing
> a ineffectual copy of X MiB via one's local machine.
i think if we're asking it to do something, it should darn well do it.
the user may well know best.
a warning would be fine, i think.
https:/
cmd/juju/
tools.ErrNoTools:
On 2013/04/16 08:43:40, fwereade wrote:
> On 2013/04/15 13:48:21, rog wrote:
> > still pained by this.
> Each non-test file that uses them together also needs to differentiate
between
> the two situations.
the code above demonstrates that statement isn't actually true.
taking a quick look through all the places that use the tools.Err*
values, i don't see any where the errors are particularly useful, and
sometimes they make things more obscure.
to try and check that i wasn't on crack, i rustled up a branch which did
what i'm suggesting (lose the special errors and the major-version
argument to ListTools). overall (not counting the tests, which i didn't
touch) it's 27 less lines of code and sync-tools will now work with all
tools, not just the same major version.
in particular, putting the major-version matching inside Match seems
like the Right Thing to do.
lp:~rogpeppe/juju-core/fwereade-less-errors
https:/