Code review comment for lp:~cmars/juju-core/no-default-series

Revision history for this message
William Reade (fwereade) wrote :

LGTM, I think these are all trivials

https://codereview.appspot.com/76860044/diff/120001/charm/url_test.go
File charm/url_test.go (right):

https://codereview.appspot.com/76860044/diff/120001/charm/url_test.go#newcode167
charm/url_test.go:167: c.Assert(err.Error(), gc.Equals,
charm.ErrUnresolvedUrl.Error())
We'd usually check for direct equality with ErrUnresolvedUrl, and
otherwise use gc.ErrorMatches to check the string.

https://codereview.appspot.com/76860044/diff/120001/charm/url_test.go#newcode213
charm/url_test.go:213: {charm.IsValidSeries, "pre-c1se", true},
Explicit test that precise-1 is not allowed, but precise1 is?

https://codereview.appspot.com/76860044/diff/120001/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (left):

https://codereview.appspot.com/76860044/diff/120001/state/apiserver/client/client_test.go#oldcode663
state/apiserver/client/client_test.go:663: // TODO(fwereade) make these
errors consistent one day.
If these errors are starting to look nicer, please drop the comment (and
the one below). If they're not, would you upgrade this to a tech-debt
bug and fill in the usual TODO template (name, date, bug#, \n, short
description)?

https://codereview.appspot.com/76860044/diff/120001/store/server.go
File store/server.go (right):

https://codereview.appspot.com/76860044/diff/120001/store/server.go#newcode80
store/server.go:80: return &charm.URL{Reference: ref, Series:
DefaultSeries}
I'm not seeing where this is called. Dead code?

https://codereview.appspot.com/76860044/

« Back to merge proposal