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

Revision history for this message
Dave Cheney (dave-cheney) wrote :

Some minor comments.

A larger meta comment, a lot of this change is noise due to charm.URL
being treated as a pointer where it is actually a value.

If all the references to *charm.URL were replaced with charm.URL the
amount of defensive coding in this change, and elsewhere would be
reduced.

https://codereview.appspot.com/76860044/diff/60001/charm/repo.go
File charm/repo.go (right):

https://codereview.appspot.com/76860044/diff/60001/charm/repo.go#newcode154
charm/repo.go:154: curl = &*curl
I don't understand what this line does. I think it does nothing. If if
does something then it need a comment to explain the magic.

https://codereview.appspot.com/76860044/diff/60001/charm/repo.go#newcode429
charm/repo.go:429: result := &*curl
yuk, how about

result := *curl
if !result ... {

}
return &result, nil

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

https://codereview.appspot.com/76860044/diff/60001/store/server.go#newcode80
store/server.go:80: curl.Series = "precise"
please make this a constant at the top of the file with a comment
explaining the effect of changing the constant.

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

« Back to merge proposal