charm: add branch location/charm url translation

charm: add branch location/charm url translation



https://code.launchpad.net/~niemeyer/juju-core/branch-urls/+merge/157098

Requires: https://code.launchpad.net/~niemeyer/juju-core/branch-handling/+merge/156976

(do not edit description out of merge proposal)

[niemeyer, 2013-04-04 12:50:56]

Please take a look.

[dimitern, 2013-04-04 13:05:39]

LGTM modulo some trivials.

https://codereview.appspot.com/8355044/diff/1/charm/repo.go
File charm/repo.go (right):

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode144
charm/repo.go:144: // BranchLocation returns the location for the branch holding
the charm at curl.
s/curl/the given charm URL/ ?

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode166
charm/repo.go:166: // CharmURL returns the charm URL for the branch at location.
s/location/the given location/?

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode191
charm/repo.go:191: return nil, fmt.Errorf("unknown branch location: %q",
location)
Errorf("charm: ...

[niemeyer, 2013-04-04 13:08:35]

https://codereview.appspot.com/8355044/diff/1/charm/repo.go
File charm/repo.go (right):

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode144
charm/repo.go:144: // BranchLocation returns the location for the branch holding
the charm at curl.
On 2013/04/04 13:05:39, dimitern wrote:
> s/curl/the given charm URL/ ?

It's common practice to reference the parameters explicitly as I'm doing here.
See Latest above.

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode166
charm/repo.go:166: // CharmURL returns the charm URL for the branch at location.
On 2013/04/04 13:05:39, dimitern wrote:
> s/location/the given location/?

Same.

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode191
charm/repo.go:191: return nil, fmt.Errorf("unknown branch location: %q",
location)
On 2013/04/04 13:05:39, dimitern wrote:
> Errorf("charm: ...

As far as I know we don't use prefixes in error messages.

[dimitern, 2013-04-04 14:07:53]

Fair enough for the parameters docs, it a personal preference I guess, fine by
me.

https://codereview.appspot.com/8355044/diff/1/charm/repo.go
File charm/repo.go (right):

https://codereview.appspot.com/8355044/diff/1/charm/repo.go#newcode191
charm/repo.go:191: return nil, fmt.Errorf("unknown branch location: %q",
location)
On 2013/04/04 13:08:35, niemeyer wrote:
> On 2013/04/04 13:05:39, dimitern wrote:
> > Errorf("charm: ...
> 
> As far as I know we don't use prefixes in error messages.

Sorry, I must be blind :) Sure, you're right.

[TheMue, 2013-04-05 12:46:04]

LGTM

[niemeyer, 2013-04-05 20:09:42]

*** Submitted:

charm: add branch location/charm url translation

R=dimitern, TheMue
CC=
https://codereview.appspot.com/8355044