Almost there and nice! Some minor suggestions, but big +1 on fwereade's comments about changing jujutest.
https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go File environs/dummy/environs.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go#newcode440 environs/dummy/environs.go:440: return fmt.Errorf("Provider storage is not writeable.") s/Provider/provider/ and no dot at the end.
https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go File environs/ec2/ec2.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go#newcode268 environs/ec2/ec2.go:268: return fmt.Errorf("Provider storage is not writeable.") On 2013/06/04 12:42:52, mue wrote: > Lowercase, "provider storage ...", and not dot at the end.
+1
https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go File environs/maas/environ.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go#newcode117 environs/maas/environ.go:117: return fmt.Errorf("Provider storage is not writeable.") On 2013/06/04 12:42:52, mue wrote: > Dito, lowercase and dot.
https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go File environs/openstack/provider.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go#newcode494 environs/openstack/provider.go:494: return fmt.Errorf("Provider storage is not writeable.") On 2013/06/04 12:42:52, mue wrote: > And again lowercase and dot.
https://codereview.appspot.com/9876043/diff/4001/environs/storage.go File environs/storage.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode18 environs/storage.go:18: const VERIFICATION_FILENAME string = "bootstrap-verify" Please, this is not C :) const VerificationFilename string = "bootstrap-verify"
https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19 environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage writing verified: ok" ditto: VerificationContent
https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go File environs/storage_test.go (right):
https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode36 environs/storage_test.go:36: type MockStorage struct { does it have to be exported?
https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode44 environs/storage_test.go:44: "Put('%s', '%s', %d)", filename, content, length) if you're doing the call multiline, please: log_message := fmt.Sprintf( "...", filename, content, length, )
https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode67 environs/storage_test.go:67: type VerifyStorageSuite struct{} again, the suite doesn't need to be exported (verifyStorageSuite or even just storageSuite)
https://codereview.appspot.com/9876043/
« Back to merge proposal
Almost there and nice! Some minor suggestions, but big +1 on fwereade's
comments about changing jujutest.
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ dummy/environs. go dummy/environs. go (right):
File environs/
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ dummy/environs. go#newcode440 dummy/environs. go:440: return fmt.Errorf( "Provider storage is provider/ and no dot at the end.
environs/
not writeable.")
s/Provider/
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ ec2/ec2. go
File environs/ec2/ec2.go (right):
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ ec2/ec2. go#newcode268 ec2/ec2. go:268: return fmt.Errorf( "Provider storage is not
environs/
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Lowercase, "provider storage ...", and not dot at the end.
+1
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ maas/environ. go maas/environ. go (right):
File environs/
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ maas/environ. go#newcode117 maas/environ. go:117: return fmt.Errorf( "Provider storage is not
environs/
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Dito, lowercase and dot.
+1
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ openstack/ provider. go openstack/ provider. go (right):
File environs/
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ openstack/ provider. go#newcode494 openstack/ provider. go:494: return fmt.Errorf( "Provider storage
environs/
is not writeable.")
On 2013/06/04 12:42:52, mue wrote:
> And again lowercase and dot.
+1
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage. go
File environs/storage.go (right):
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage. go#newcode18 storage. go:18: const VERIFICATION_ FILENAME string = ename string = "bootstrap-verify"
environs/
"bootstrap-verify"
Please, this is not C :)
const VerificationFil
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage. go#newcode19 storage. go:19: const VERIFICATION_ CONTENT = "juju-core storage
environs/
writing verified: ok"
ditto: VerificationContent
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage_ test.go storage_ test.go (right):
File environs/
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage_ test.go# newcode36 storage_ test.go: 36: type MockStorage struct {
environs/
does it have to be exported?
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage_ test.go# newcode44 storage_ test.go: 44: "Put('%s', '%s', %d)", filename, content,
environs/
length)
if you're doing the call multiline, please:
log_message := fmt.Sprintf(
"...",
filename,
content,
length,
)
https:/ /codereview. appspot. com/9876043/ diff/4001/ environs/ storage_ test.go# newcode67 storage_ test.go: 67: type VerifyStorageSuite struct{}
environs/
again, the suite doesn't need to be exported (verifyStorageSuite or even
just storageSuite)
https:/ /codereview. appspot. com/9876043/