Code review comment for lp:~axwalk/juju-core/lp1271144-maas-bridge-utils

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/03/20 09:42:15, rog wrote:
> LGTM with a couple of thoughts below.
> File provider/maas/environ.go (right):
> provider/maas/environ.go:316:
> utils.CommandString(utils.AptGetCommand("update")...),
> It seems a pity that we can't just call SetAptUpdate on the
> cloudinit config, but that would require a change to ComposeUserData
> (might it make sense to pass in a possibly-nil cloudinit.Config to
> function?)

> Aside: CommandString should also escape backquotes, although it won't
be a
> problem here
> File provider/maas/environ_test.go (right):
> provider/maas/environ_test.go:208: const aptGetPrefix = "env
> DEBIAN_FRONTEND=noninteractive apt-get
> --option=Dpkg::options::=--force-unsafe-io --assume-yes --quiet"
> or aptGetPrefix := utils.AptGetCommand()
> ?

Except you also have to flatten, and I'd rather avoid using the code
that's used by the thing being tested.

« Back to merge proposal