Code review comment for lp:~dave-cheney/juju-core/167-simplestreams-add-trusty

On 2014/02/11 04:02:03, dfc wrote:
> Lost in the various commit messages was a desire to consolidate the
list of
> supported series. This proposal is the first of several.

> On Tue, Feb 11, 2014 at 2:51 PM, <mailto:<email address hidden>> wrote:

> > Reviewers: http://mp+205684_code.launchpad.net,
> >
> > Message:
> > Please take a look.
> >
> > Description:
> > simplestreams: add trusty
> >
> >
> >
> > https://code.launchpad.net/~dave-cheney/juju-core/167-
> > simplestreams-add-trusty/+merge/205684
> >
> > (do not edit description out of merge proposal)
> >
> >
> > Please review this at https://codereview.appspot.com/61560045/
> >
> > Affected files (+9, -4 lines):
> > A [revision details]
> > M environs/simplestreams/simplestreams.go
> > M environs/simplestreams/simplestreams_test.go
> > M testing/constants.go
> >
> >
> > Index: [revision details]
> > === added file '[revision details]'
> > --- [revision details] 2012-01-01 00:00:00 +0000
> > +++ [revision details] 2012-01-01 00:00:00 +0000
> > @@ -0,0 +1,2 @@
> > +Old revision: tarmac-20140210142230-bs4gcvlb7cjb3tig
> > +New revision:
mailto:<email address hidden>
> >
> > Index: testing/constants.go
> > === modified file 'testing/constants.go'
> > --- testing/constants.go 2013-07-30 16:36:03 +0000
> > +++ testing/constants.go 2014-02-11 03:40:13 +0000
> > @@ -25,3 +25,6 @@
> > Total: LongWait,
> > Delay: ShortWait,
> > }
> > +
> > +// SupportedSeries lists the series known to Juju.
> > +var SupportedSeries = []string{"precise", "quantal", "raring",
"saucy",
> > "trusty"}
> >
> >
> > Index: environs/simplestreams/simplestreams.go
> > === modified file 'environs/simplestreams/simplestreams.go'
> > --- environs/simplestreams/simplestreams.go 2014-01-29 09:58:08
+0000
> > +++ environs/simplestreams/simplestreams.go 2014-02-11 03:40:13
+0000
> > @@ -127,8 +127,8 @@
> > seriesVersionsMutex.Lock()
> > defer seriesVersionsMutex.Unlock()
> > updateSeriesVersions()
> > - series := []string{}
> > - for s, _ := range seriesVersions {
> > + var series []string
> > + for s := range seriesVersions {
> > series = append(series, s)
> > }
> > return series
> >
> >
> > Index: environs/simplestreams/simplestreams_test.go
> > === modified file 'environs/simplestreams/simplestreams_test.go'
> > --- environs/simplestreams/simplestreams_test.go 2013-11-26
> > 12:24:48 +0000
> > +++ environs/simplestreams/simplestreams_test.go 2014-02-11
> > 03:40:13 +0000
> > @@ -13,6 +13,7 @@
> >
> > "launchpad.net/juju-core/environs/simplestreams"
> > sstesting
"launchpad.net/juju-core/environs/simplestreams/testing"
> > + coretesting "launchpad.net/juju-core/testing"
> > jc "launchpad.net/juju-core/testing/checkers"
> > )
> >
> > @@ -411,8 +412,7 @@
> > defer cleanup()
> > series := simplestreams.SupportedSeries()
> > sort.Strings(series)
> > - series = series[0:4]
> > - c.Assert(series, gc.DeepEquals, []string{"precise",
"quantal",
> > "raring", "saucy"})
> > + c.Assert(series, gc.DeepEquals, coretesting.SupportedSeries)
> > }
> >
> > var getMirrorTests = []struct {
> >
> >
> >
> >
> >

LGTM. This will cause our tests to fail when ubuntu.csv is updated, and
I think that's a good thing. We shouldn't have production code and tests
having a different idea about which series are supported.

https://codereview.appspot.com/61560045/