Code review comment for lp://staging/~fwereade/juju-core/environs-tools-provisioning

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

https://codereview.appspot.com/8727044/diff/1/environs/tools.go#newcode137
environs/tools.go:137: // environment's configuration.
On 2013/04/16 02:29:56, fwereade wrote:
> On 2013/04/15 11:12:13, rog wrote:
> > given the name of this function, i'm not sure that this casual
side-effect is
> > great.
> >
> > perhaps it might work better if we made the side effect the
principal purpose
> of
> > the function.
> >
> > something like this, perhaps?
> >
> > // ChooseAgentVersion makes sure that the
> > // environment's agent version is set to a value
> > // that is compatible with the tools in the
> > // environment's storage, given the supplied constraints,
> > // setting it if necessary.
> > //
> > // It returns the non-empty set of tools with that version.
> > func ChooseAgentVersion(environ Envirom, con constraints.Value)
(list
> > tools,List, err error)

> I'm not sure we came to any firm conclusions when we discussed this on
IRC. I'm
> starting to ponder putting all this logic quietly inside Bootstrap()
and passing
> possibleTools on down to Environ.Bootstrap()... but for now I call
> progress-not-perfection.

please, please change the name so it expresses the fact that it has a
significant side effect. i know you're not keen on ChangeAgentVersion
(or, presumably EnsureAgentVersion or SetAgentVersion) but something
different from FindBootstrapTools would make me happier.

https://codereview.appspot.com/8727044/