Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Choose series for machines based on charm/default series #20
Conversation
|
Test PASSed. |
QA
|
frankban
reviewed
Apr 6, 2016
| @@ -1197,6 +1255,107 @@ var fromDataTests = []struct { | ||
| }, | ||
| Requires: []string{"addCharm-0"}, | ||
| }}, | ||
| +}, { | ||
| + about: "service with series", |
frankban
reviewed
Apr 6, 2016
| @@ -28,9 +28,17 @@ func handleServices(add func(Change), services map[string]*charm.ServiceSpec) ma | ||
| service := services[name] | ||
| // Add the addCharm record if one hasn't been added yet. | ||
| if charms[service.Charm] == "" { | ||
| + series := service.Series | ||
| + if series == "" { | ||
| + serviceCharm := charm.MustParseURL(service.Charm) |
frankban
Apr 6, 2016
Member
Just u := charm.MustParseURL(service.Charm)?
And please add a comment stating that this is safe because the bundle data is assumed to be already verified, and therefore this must be a valid charm URL.
frankban
reviewed
Apr 6, 2016
| @@ -86,9 +94,13 @@ func handleMachines(add func(Change), machines map[string]*charm.MachineSpec) ma | ||
| if machine == nil { | ||
| machine = &charm.MachineSpec{} | ||
| } | ||
| + var series = machine.Series |
frankban
reviewed
Apr 6, 2016
| @@ -172,9 +184,17 @@ func handleUnits(add func(Change), services map[string]*charm.ServiceSpec, added | ||
| if i < numPlaced { | ||
| p = service.To[i] | ||
| } | ||
| + var series = service.Series |
frankban
reviewed
Apr 6, 2016
| @@ -172,9 +184,17 @@ func handleUnits(add func(Change), services map[string]*charm.ServiceSpec, added | ||
| if i < numPlaced { | ||
| p = service.To[i] | ||
| } | ||
| + var series = service.Series | ||
| + if series == "" { | ||
| + serviceCharm := charm.MustParseURL(service.Charm) |
frankban
Apr 6, 2016
Member
Same comment here. Since this logic is repeated twice, is it worth an external function? Like
func getSeries(service *charm.ServiceSpec, defaultSeries string) string {
if service.Series != "" {
return service.Series
}
// The following is safe because the bundle data is assumed to be already
// verified, and therefore this must be a valid charm URL.
series := charm.MustParseURL(service.Charm).Series
if series != "" {
return series
}
return defaultSeries
}
|
|
|
Test PASSed. |
|
Test PASSed. |
|
|
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/bundlechanges-merge |
makyo commentedApr 6, 2016
Addresses https://bugs.launchpad.net/juju-core/+bug/1564057 - will require a dependencies update in core.