Code review comment for lp:~fwereade/pyjuju/go-charm-resolve

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

*** Submitted:

add charm.InferRepository

R=niemeyer
CC=
https://codereview.appspot.com/6261058

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

https://codereview.appspot.com/6261058/diff/1/charm/charm.go#newcode32
charm/charm.go:32: // a charm matching that URL.
On 2012/06/01 13:32:34, niemeyer wrote:
> // InferRepository returns a charm repository and URL inferred from
the provided
> // parameters. charmAlias may hold an exact charm URL, or an alias in
a
> // format supported by InferURL.

Done.

https://codereview.appspot.com/6261058/diff/1/charm/charm.go#newcode33
charm/charm.go:33: func Resolve(name, repoPath, defaultSeries string)
(repo Repository, curl *URL, err error) {
On 2012/06/01 13:32:34, niemeyer wrote:
> This is InferURL + repository. I suggest this signature:

> func InferRepository(charmAlias, defaultSeries, localRepoPath string)
(repo
> Repository, curl *URL, err error)

> (note the parameter ordering matching InferURL)

Done.

https://codereview.appspot.com/6261058/diff/1/charm/charm.go#newcode41
charm/charm.go:41: repo = &LocalRepository{repoPath}
On 2012/06/01 13:32:34, niemeyer wrote:
> Should we support the case where localRepoPath is ""? Not sure about
how the
> call site for this will look like.

I think an explicit error here is a good idea. Done.

https://codereview.appspot.com/6261058/

« Back to merge proposal