Code review comment for lp:~frankban/juju-core/get-charm-api

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM, thanks for doing this!

https://codereview.appspot.com/67750045/diff/1/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/67750045/diff/1/state/api/params/internal.go#newcode509
state/api/params/internal.go:509: // CharmsResponse is the server
response to charm upload or get requests.
s/get/GET/

https://codereview.appspot.com/67750045/diff/1/state/apiserver/charms.go
File state/apiserver/charms.go (right):

https://codereview.appspot.com/67750045/diff/1/state/apiserver/charms.go#newcode540
state/apiserver/charms.go:540: if !strings.HasPrefix(filePath,
bundlePath+"/") {
Please, use filepath.Dir(filePath) != bundlePath here instead.

https://codereview.appspot.com/67750045/diff/1/state/apiserver/charms_test.go
File state/apiserver/charms_test.go (right):

https://codereview.appspot.com/67750045/diff/1/state/apiserver/charms_test.go#newcode249
state/apiserver/charms_test.go:249: "expected url=CharmURL query
argument")
I'd prefer if you formatted this and other places where
assertErrorResponse is too long, like this:

s.assertErrorResponse(
     c, resp, http.StatusBadRequest,
     expected url=CharmURL query argument",
)

https://codereview.appspot.com/67750045/

« Back to merge proposal