Code review comment for lp:~axwalk/juju-core/lp1247232-bootstrap-majorminorpatch

On 2014/04/17 09:45:47, jameinel wrote:
> I'd really like to see this tested live, though I'm not sure it will
be easy to
> set up the test (you'll have to populate a couple versions of the
tools, and
> then bootstrap with an old version of them).

I did test it live by applying the patch to 1.17.6. It successfully
bootstrapped with 1.17.6, and then immediately upgraded to 1.17.7.

> We'll also certainly want to let CI know about it right away, since it
will
> change how they do some of their testing.

> Mostly, though, LGTM.

https://codereview.appspot.com/88840043/diff/1/environs/bootstrap/bootstrap.go
> File environs/bootstrap/bootstrap.go (right):

https://codereview.appspot.com/88840043/diff/1/environs/bootstrap/bootstrap.go#newcode72
> environs/bootstrap/bootstrap.go:72: // We should only ever bootstrap
the exact
> same version as the client,
> This whole chunk of code seems quite localized, is it possible to just
pull it
> into a helper function to keep the units of work focused?

I pulled out a little bit, but half of it is specific to the context.

https://codereview.appspot.com/88840043/