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

Issue 6851081: environs/ec2: respect default series (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 4 months ago by dfc
Modified:
11 years, 3 months ago
Reviewers:
rog, jameinel, mp+135070, niemeyer
Visibility:
Public.

Description

environs/ec2: respect default series Fix bootstrapping from Quantal to Precise, and vise versa via the default-series configuration flag. Fixes is #1074064 This only works for *released* toolsets, which have been prepared on a series native machine. For development toolsets, defaul-series must not be present in your configuration, as the toolset will always be generated based on the current series of the machine you are working on. To be clear, if you develop on precise, --upload-tools will only produce precise tools, for quantal, quantal tools. https://code.launchpad.net/~dave-cheney/juju-core/050-environs-respect-default-series-II/+merge/135070 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : environs/ec2: respect default series #

Patch Set 3 : environs/ec2: respect default series #

Total comments: 3

Patch Set 4 : environs/ec2: respect default series #

Patch Set 5 : environs/ec2: respect default series #

Total comments: 6

Patch Set 6 : environs/ec2: respect default series #

Total comments: 1

Patch Set 7 : environs/ec2: respect default series #

Total comments: 1

Patch Set 8 : environs/ec2: respect default series #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -9 lines) Patch
A [revision details] View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M environs/ec2/ec2.go View 1 2 3 4 5 6 7 1 chunk +3 lines, -1 line 2 comments Download
M environs/ec2/live_test.go View 1 2 3 4 5 6 7 1 chunk +5 lines, -0 lines 0 comments Download
M environs/ec2/local_test.go View 1 2 3 4 5 6 7 4 chunks +34 lines, -8 lines 0 comments Download
M version/version.go View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 13
dfc
Please take a look.
11 years, 4 months ago (2012-11-20 07:11:19 UTC) #1
dfc
Please take a look.
11 years, 4 months ago (2012-11-20 07:12:48 UTC) #2
niemeyer
Mostly great, although it's not entirely clear why the live tests are hacking the current ...
11 years, 4 months ago (2012-11-20 22:15:46 UTC) #3
dfc
Please take a look.
11 years, 4 months ago (2012-11-21 00:09:38 UTC) #4
rog
LGTM, although i'm not *quite* sure what logic we're testing by setting default-series to different ...
11 years, 4 months ago (2012-11-21 10:27:04 UTC) #5
dfc
Please take a look. https://codereview.appspot.com/6851081/diff/9002/environs/ec2/live_test.go File environs/ec2/live_test.go (right): https://codereview.appspot.com/6851081/diff/9002/environs/ec2/live_test.go#newcode43 environs/ec2/live_test.go:43: // other series'. On 2012/11/21 ...
11 years, 4 months ago (2012-11-21 11:42:48 UTC) #6
niemeyer
I'm sorry for not bringing better news this time, but it still doesn't feel entirely ...
11 years, 4 months ago (2012-11-21 20:40:17 UTC) #7
dfc
Please take a look.
11 years, 4 months ago (2012-11-23 01:53:27 UTC) #8
niemeyer
That's pretty much unchanged from the perspective of the last review. More detailed feedback: https://codereview.appspot.com/6851081/diff/6004/environs/ec2/local_test.go ...
11 years, 4 months ago (2012-11-26 19:49:02 UTC) #9
dfc
*** abandoned ***
11 years, 4 months ago (2012-11-26 23:58:14 UTC) #10
niemeyer
On 2012/11/26 23:58:14, dfc wrote: > *** abandoned *** Why was it abandoned? The change ...
11 years, 4 months ago (2012-11-27 11:48:32 UTC) #11
jameinel
I think if we pull the code out we can test it more easily. Otherwise ...
11 years, 3 months ago (2012-12-05 08:09:55 UTC) #12
niemeyer
11 years, 3 months ago (2012-12-05 11:25:26 UTC) #13
https://codereview.appspot.com/6851081/diff/7004/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/6851081/diff/7004/environs/ec2/ec2.go#newcode242
environs/ec2/ec2.go:242: tools, err = environs.FindTools(e, vers, flags)
On 2012/12/05 08:09:55, jameinel wrote:
> Isn't this changing the Series on the version.Current object? Shouldn't this
> either be set somewhere else or we should be creating a copy of the Current
> object?

It does create a copy. foo := bar always copies bar. If bar is a pointer, the
pointer is copied. If it's a value, the value is copied.
Sign in to reply to this message.

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