Code review comment for lp:~niemeyer/juju-core/resource-not-found

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

*** Submitted:

state: getter methods return NotFoundError

Due to the in-progress lifecycle changes, client code
must be able to distinguish not found errors.

R=dfc, TheMue, rog
CC=
https://codereview.appspot.com/6549056

https://codereview.appspot.com/6549056/diff/2002/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6549056/diff/2002/state/state.go#newcode46
state/state.go:46: format string
On 2012/09/24 14:12:04, rog wrote:
> i'm not sure that i see the benefit in storing the format and args
here. are we
> really concerned that the work involved in formatting the error
message is going
> to slow us down?

I certainly don't care enough to argue. I'll change it.

https://codereview.appspot.com/6549056/diff/2002/state/state.go#newcode53
state/state.go:53:
On 2012/09/24 05:20:35, dfc wrote:
> comment please.

I honestly think it's not worth it in this case. Reading the body of the
function is more clear and takes less time than reading any comments we
add here, and since it's a private function it won't show up in the
docs.

https://codereview.appspot.com/6549056/diff/2002/state/state.go#newcode55
state/state.go:55: return &NotFoundError{format, args}
On 2012/09/24 09:37:11, TheMue wrote:
> If no later access to the args is planned the error message could
already be

Changed so NotFoundError has a dumb msg argument.

https://codereview.appspot.com/6549056/diff/2002/state/state.go#newcode384
state/state.go:384: func (s *State) Relation(endpoints
...RelationEndpoint) (r *Relation, err error) {
On 2012/09/24 05:20:35, dfc wrote:
> suggestion: drop the named return values as we're not using defer
ErrorContextf
> anymore in this method.

Done.

https://codereview.appspot.com/6549056/

« Back to merge proposal