LGTM with a few suggestions below. Nice!
https://codereview.appspot.com/50090044/diff/1/state/api/params/internal.go File state/api/params/internal.go (right):
https://codereview.appspot.com/50090044/diff/1/state/api/params/internal.go#newcode115 state/api/params/internal.go:115: // environment. // EnvironmentResult holds the result of an API call returning a name and UUID for an environment ?
Also s/CurrentEnvironmentResult/EnvironmentResult/ ?
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/environ.go File state/api/uniter/environ.go (left):
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/environ.go#oldcode23 state/api/uniter/environ.go:23: // TODO(dimitern): 2013-09-06 bug 1221834 Can you update the bug to exclude Environ.UUID() from the list, because with this change I think we can consider it cached.
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/environ.go File state/api/uniter/environ.go (right):
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/environ.go#newcode9 state/api/uniter/environ.go:9: // Environment represents the name and the unique identifier of an environment. No need to change this comment I think - see other examples in api/uniter.
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/uniter.go File state/api/uniter/uniter.go (right):
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/uniter.go#newcode164 state/api/uniter/uniter.go:164: if err != nil { you need to check result.Error as well, in addition to checking err - see APIAddresses below.
https://codereview.appspot.com/50090044/diff/1/state/api/uniter/uniter.go#newcode173 state/api/uniter/uniter.go:173: func (st *State) environment1dot16() (*Environment, error) { // environment1dot16 requests just the UUID of the current environment, when using an older apiserver that does not support CurrentEnvironment API call. ?
https://codereview.appspot.com/50090044/
« Back to merge proposal
LGTM with a few suggestions below. Nice!
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ params/ internal. go params/ internal. go (right):
File state/api/
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ params/ internal. go#newcode115 params/ internal. go:115: // environment.
state/api/
// EnvironmentResult holds the result of an API call returning a name
and UUID for an environment ?
Also s/CurrentEnviro nmentResult/ EnvironmentResu lt/ ?
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ environ. go uniter/ environ. go (left):
File state/api/
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ environ. go#oldcode23 uniter/ environ. go:23: // TODO(dimitern): 2013-09-06 bug
state/api/
1221834
Can you update the bug to exclude Environ.UUID() from the list, because
with this change I think we can consider it cached.
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ environ. go uniter/ environ. go (right):
File state/api/
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ environ. go#newcode9 uniter/ environ. go:9: // Environment represents the name and
state/api/
the unique identifier of an environment.
No need to change this comment I think - see other examples in
api/uniter.
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ uniter. go uniter/ uniter. go (right):
File state/api/
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ uniter. go#newcode164 uniter/ uniter. go:164: if err != nil {
state/api/
you need to check result.Error as well, in addition to checking err -
see APIAddresses below.
https:/ /codereview. appspot. com/50090044/ diff/1/ state/api/ uniter/ uniter. go#newcode173 uniter/ uniter. go:173: func (st *State) environment1dot16()
state/api/
(*Environment, error) {
// environment1dot16 requests just the UUID of the current environment,
when using an older apiserver that does not support CurrentEnvironment
API call. ?
https:/ /codereview. appspot. com/50090044/