|
|
|
environs/config: use "" instead of nil
As discussed on line.
Tests now pass with inaccessible ~/.ssh and ~/.juju.
https://code.launchpad.net/~rogpeppe/juju-core/168-config-fix-hopefully/+merge/135956
(do not edit description out of merge proposal)
Patch Set 1 #Patch Set 2 : environs/config: use "" instead of nil #Patch Set 3 : environs/config: use "" instead of nil #Patch Set 4 : environs/config: use "" instead of nil #Patch Set 5 : environs/config: use "" instead of nil #
Total comments: 5
Patch Set 6 : environs/config: use "" instead of nil #
Total messages: 6
|
rog
Please take a look.
|
13 years, 1 month ago (2012年11月23日 18:03:28 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
Please take a look.
Please take a look.
LGTM, thanks! Please just revert the gofmt changes on bootstrap.go, as we discussed.
*** Submitted: environs/config: use "" instead of nil As discussed on line. Tests now pass with inaccessible ~/.ssh and ~/.juju. R=niemeyer CC= https://codereview.appspot.com/6854088
Having "" as the special value doesn't really appeal, but seems an obvious improvement over nil if it actually needs to be specified in the config. https://codereview.appspot.com/6854088/diff/10001/environs/dummy/environs_tes... File environs/dummy/environs_test.go (right): https://codereview.appspot.com/6854088/diff/10001/environs/dummy/environs_tes... environs/dummy/environs_test.go:18: "authorized-keys": "foo", I prefer something like 'an-ssh-key' for the testing value, easier to grep for than 'foo' if you see it in test error output. https://codereview.appspot.com/6854088/diff/10001/environs/ec2/local_test.go File environs/ec2/local_test.go (right): https://codereview.appspot.com/6854088/diff/10001/environs/ec2/local_test.go#... environs/ec2/local_test.go:37: "authorized-keys": "foo", Again, 'an-ssh-key' or similar. https://codereview.appspot.com/6854088/diff/10001/environs/openstack/config_t... File environs/openstack/config_test.go (right): https://codereview.appspot.com/6854088/diff/10001/environs/openstack/config_t... environs/openstack/config_test.go:56: "authorized-keys": "foo", Likewise. https://codereview.appspot.com/6854088/diff/10001/environs/openstack/local_te... File environs/openstack/local_test.go (right): https://codereview.appspot.com/6854088/diff/10001/environs/openstack/local_te... environs/openstack/local_test.go:28: "authorized-keys": "foo", Likewise. https://codereview.appspot.com/6854088/diff/10001/juju/bootstrap.go File juju/bootstrap.go (right): https://codereview.appspot.com/6854088/diff/10001/juju/bootstrap.go#newcode78 juju/bootstrap.go:78: MaxPathLen: 0, // Disallow delegation for now. Heh, same go fmt weirdness as the bootstrap move branch. At least bzr should be kind to you over resolving conflicts.