Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(49)

Issue 6261058: add charm.InferRepository

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years ago by fwereade
Modified:
9 years ago
Reviewers:
mp+108337
Visibility:
Public.

Description

add charm.InferRepository https://code.launchpad.net/~fwereade/juju/go-charm-resolve/+merge/108337 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 6

Patch Set 2 : add charm.InferRepository #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -1 line) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M charm/charm.go View 1 2 chunks +26 lines, -1 line 0 comments Download
M charm/charm_test.go View 1 1 chunk +30 lines, -0 lines 0 comments Download

Messages

Total messages: 3
fwereade
Please take a look.
9 years ago (2012-06-01 13:03:56 UTC) #1
niemeyer
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 ...
9 years ago (2012-06-01 13:32:33 UTC) #2
fwereade
9 years ago (2012-06-01 14:32:33 UTC) #3
*** 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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b