Code review comment for lp://staging/~wallyworld/juju-core/simplestreams-multiple-indices

Revision history for this message
Ian Booth (wallyworld) wrote :

https://codereview.appspot.com/9128047/diff/7001/environs/imagemetadata/simplestreams.go
File environs/imagemetadata/simplestreams.go (right):

https://codereview.appspot.com/9128047/diff/7001/environs/imagemetadata/simplestreams.go#newcode212
environs/imagemetadata/simplestreams.go:212: if _, ok :=
err.(*environs.NotFoundError); ok {
On 2013/05/21 16:07:22, TheMue wrote:
> On 2013/05/17 01:36:03, wallyworld wrote:
> > On 2013/05/16 06:47:13, jameinel wrote:
> > > generally I really prefer the:
> > >
> > > environs.IsNotFound(err)
> > >
> > > method. This still works, but I'm wondering if you prefer it?
> >
> > Sadly only goose has nice error helpers like that

> IsXyzError() looks better, yes. So it should be added to each own
error type.
> But in case it returns true sometimes the asserted type is needed (if
it
> contains more information than just a string). And then still a
construct
> similar to the one above is added (not in this case, as the asserted
error isn't
> used directly).

I've added an environs.IsNotFoundError() method and replaced all
occurrences of
if _, ok := err.(*environs.NotFoundError); ok {
}
with
if environs.IsNotFoundError(err) {
}

(there were about 7 of them)
The asserted NotFoundError is not used anywhere so the simple bool
method is all that is needed.

https://codereview.appspot.com/9128047/

« Back to merge proposal