this is looking nice. a few minor remarks below.
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go File cmd/jujud/provisioning.go (right):
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go#newcode17 cmd/jujud/provisioning.go:17: const PROVIDER_MACHINE_ID = "provider-machine-id" s/PROVIDER_MACHINE_ID/providerMachineId/
UNDERSCORED_CAPS aren't conventional.
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go#newcode120 cmd/jujud/provisioning.go:120: return fmt.Errorf("machine-%010d already reports a provider id %q, skipping", m.Id(), id) s/machine-%010d/machine %d/
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go#newcode208 cmd/jujud/provisioning.go:208: return "", fmt.Errorf("findProviderId: machine-%010d key not found: %q", m.Id(), PROVIDER_MACHINE_ID) i think 'machine %d' would work better - the %010 stuff is detail internal to state.
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go#newcode210 cmd/jujud/provisioning.go:210: if _, ok := id.(string); !ok { rather than doing the type conversion twice, perhaps:
if id, ok := id.(string); ok { return id, nil } return "", fmt.Errorf("machine %d has invalid value for %s: %#v", m.Id(), PROVIDER_MACHINE_ID, id)
https://codereview.appspot.com/6250068/diff/2001/cmd/jujud/provisioning.go#newcode221 cmd/jujud/provisioning.go:221: if len(insts) < 1 { this can't happen.
https://codereview.appspot.com/6250068/
« Back to merge proposal
this is looking nice.
a few minor remarks below.
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go provisioning. go (right):
File cmd/jujud/
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go#newcode17 provisioning. go:17: const PROVIDER_MACHINE_ID = machine- id" MACHINE_ ID/providerMach ineId/
cmd/jujud/
"provider-
s/PROVIDER_
UNDERSCORED_CAPS aren't conventional.
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go#newcode120 provisioning. go:120: return fmt.Errorf( "machine- %010d already %010d/machine %d/
cmd/jujud/
reports a provider id %q, skipping", m.Id(), id)
s/machine-
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go#newcode208 provisioning. go:208: return "", fmt.Errorf( "findProviderId : MACHINE_ ID)
cmd/jujud/
machine-%010d key not found: %q", m.Id(), PROVIDER_
i think 'machine %d' would work better - the %010 stuff is detail
internal to state.
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go#newcode210 provisioning. go:210: if _, ok := id.(string); !ok {
cmd/jujud/
rather than doing the type conversion twice, perhaps:
if id, ok := id.(string); ok { MACHINE_ ID, id)
return id, nil
}
return "", fmt.Errorf("machine %d has invalid value for %s: %#v",
m.Id(), PROVIDER_
https:/ /codereview. appspot. com/6250068/ diff/2001/ cmd/jujud/ provisioning. go#newcode221 provisioning. go:221: if len(insts) < 1 {
cmd/jujud/
this can't happen.
https:/ /codereview. appspot. com/6250068/