Merge lp://staging/~sergiusens/snappy/rollYourOwnCorePersonal into lp://staging/~snappy-dev/snappy/snappy-moved-to-github

Proposed by Sergio Schvezov
Status: Needs review
Proposed branch: lp://staging/~sergiusens/snappy/rollYourOwnCorePersonal
Merge into: lp://staging/~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 220 lines (+50/-34)
6 files modified
cmd/snappy/cmd_info.go (+2/-11)
release/release.go (+19/-10)
release/release_test.go (+10/-10)
snappy/snapp_test.go (+1/-1)
snappy/systemimage.go (+3/-2)
snappy/systemimage_test.go (+15/-0)
To merge this branch: bzr merge lp://staging/~sergiusens/snappy/rollYourOwnCorePersonal
Reviewer Review Type Date Requested Status
Federico Gimenez (community) continuous-integration Approve
Michael Vogt (community) Needs Information
Snappy Tarmac continuous-integration Pending
Review via email: mp+264405@code.staging.launchpad.net

Commit message

Snappy to expose the core package as personal or core depending on the release

Description of the change

This will affect snappy config (and maybe oem packages that define configs) so releases for snaps may have to be split up after this

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

Thanks, this looks good! I have some question inline, probably mostly my own ignorance.

review: Needs Information
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

On Mon, Jul 13, 2015 at 07:04:25AM -0000, Michael Vogt wrote:
> Review: Needs Information
>
> Thanks, this looks good! I have some question inline, probably mostly my own ignorance.
>
> Diff comments:
>
> > === modified file 'cmd/snappy/cmd_info.go'
> > --- cmd/snappy/cmd_info.go 2015-07-01 14:56:49 +0000
> > +++ cmd/snappy/cmd_info.go 2015-07-10 11:47:46 +0000
> > @@ -90,22 +91,12 @@
> > return nil
> > }
> >
> > -func ubuntuCoreChannel() string {
> > - parts, err := snappy.ActiveSnapsByType(pkg.TypeCore)
> > - if len(parts) == 1 && err == nil {
> > - return parts[0].Channel()
> > - }
> > -
> > - return "unknown"
> > -}
> > -
> > func info() error {
> > - release := ubuntuCoreChannel()
> > frameworks, _ := snappy.ActiveSnapNamesByType(pkg.TypeFramework)
> > apps, _ := snappy.ActiveSnapNamesByType(pkg.TypeApp)
> >
> > // TRANSLATORS: the %s release string
> > - fmt.Printf(i18n.G("release: %s\n"), release)
> > + fmt.Printf(i18n.G("release: %s\n"), release.String())
>
> This is nice (i.e. that all the code above gets killed :)
>

\o/

> > // TRANSLATORS: the %s an architecture string
> > fmt.Printf(i18n.G("architecture: %s\n"), snappy.Architecture())
> > // TRANSLATORS: the %s is a comma separated list of framework names
> >
> > === modified file 'release/release.go'
> > --- release/release.go 2015-05-15 13:33:27 +0000
> > +++ release/release.go 2015-07-10 11:47:46 +0000
> > @@ -51,9 +51,18 @@
> > return rel.String()
> > }
> >
> > +// Flavor returns the flavor for the release.
> > +func Flavor() string {
> > + return rel.flavor
> > +}
> > +
> > // Override sets up the release using a Release
> > -func Override(r Release) {
> > - rel = r
> > +func Override(flavor, series, channel string) {
>
> What the purpose of this function? AFAICT its only used in systemimage_test.go. Or is it there to allow overrides in the future? If thats the case, why will we need "overrides"? It seems like the channel.ini gives us the "ubuntu-{core,personal}" information as part of its Setup (or am I missing something)?
>

You forget about u-d-f and obtaining the first oem snap that has all the
info we need ;-)

> > + rel = Release{
> > + flavor: flavor,
> > + series: series,
> > + channel: channel,
> > + }
> > }
> >
> > // Setup is used to initialiaze the release information for the system
> >
> > === modified file 'snappy/systemimage.go'
> > --- snappy/systemimage.go 2015-07-01 14:48:33 +0000
> > +++ snappy/systemimage.go 2015-07-10 11:47:46 +0000
> > @@ -97,7 +98,7 @@
> >
> > // Name returns the name
> > func (s *SystemImagePart) Name() string {
> > - return systemImagePartName
> > + return fmt.Sprintf("ubuntu-%s", release.Flavor())
>
> Yay for this! Makes me wonder if we should move the "ubuntu-" string into release as well so that systemimage.go is a bit more agonstic about (but really minor minor).
>

Maybe so, but we also need to be backwards compatible here; if we can,
I'm all for it!

> > }
> >
> > // Origin returns the origin ("ubuntu")
>
>
> --
> https://code.launchpad.net/~sergiusens/snappy/rollYourOwnCorePersonal/+merge/264405
> You are the owner of lp:~sergiusens/snappy/rollYourOwnCorePersonal.

Revision history for this message
Federico Gimenez (fgimenez) wrote :

PASSED: Continuous integration, rev:570
http://10.55.60.183:8080/job/snappy-rolling-ci/25/
Executed test runs:

Click here to trigger a rebuild:
http://10.55.60.183:8080/job/snappy-rolling-ci/25/rebuild

review: Approve (continuous-integration)

Unmerged revisions

570. By Sergio Schvezov

Snappy to expose the core package as personal or core depending on the release

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches