Code review comment for lp:~benji/juju-core/1130173

LGTM with the below ErrCode fix and other people's comments addressed.

https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go
File state/api/api_test.go (right):

https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go#newcode269
state/api/api_test.go:269: if err != nil && err.Error() == "permission
denied" {
On 2013/03/07 15:33:18, dimitern wrote:
> On 2013/03/07 14:33:21, benji wrote:
> > On 2013/03/05 22:45:27, dimitern wrote:
> > > instead of err.Error() == "permission denied", can you use like
> ErrPermDenied
> > > and compare to that? Or having a helper func IsPermDenied(taking
an error)?
> >
> > I'm afraid I don't follow. Are you objecting to the string literal
or
> something
> > else?

> Sorry, yes - the literal. It'd be nice to have this as var
ErrPermDenied =
> fmt.Errorf("permission denied") somewhere

actually, this should actually be api.ErrCode(err) ==
api.CodeUnauthorized, i think.

https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go
File state/statecmd/resolved_test.go (right):

https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode21
state/statecmd/resolved_test.go:21: // Ensure our test suite satisfies
Suite
On 2013/03/09 14:42:02, fwereade wrote:
> This is not correct: Suite is a function, and the purpose of the line
is to
> register the suite for running by gocheck.

> The following construction (as snarfed from
worker/deployer/simple.go):

> var _ Context = (*SimpleContext)(nil)

> ...is the one that ensures that a pointer to a SimpleContext satisfies
the
> Context interface (and does so at compile time).

> It seems there are several places this misunderstanding is propagated
in this
> package; please fix those too.

yeah, but understandable misunderstanding. the way that Suite returns a
value just so you can use it in this way is highly non-obvious.

https://codereview.appspot.com/7460047/