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

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

LGTM

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.
// 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.

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) {
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)

https://codereview.appspot.com/6261058/diff/1/charm/charm.go#newcode41
charm/charm.go:41: repo = &LocalRepository{repoPath}
Should we support the case where localRepoPath is ""? Not sure about how
the call site for this will look like.

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

« Back to merge proposal