[Merge] lp:~sergiusens/goget-ubuntu-touch/noverb into lp:goget-ubuntu-touch

Sergio Schvezov sergio.schvezov at canonical.com
Fri Nov 28 10:09:21 UTC 2014


On Fri, Nov 28, 2014 at 2:03 AM, Ricardo Salveti <rsalveti at rsalveti.net>
wrote:

> LGTM, besides the other block that in theory is not part of this MR.
>
> Diff comments:
>
> > === modified file 'diskimage/image.go'
> > --- diskimage/image.go        2014-11-25 19:31:44 +0000
> > +++ diskimage/image.go        2014-11-26 19:44:54 +0000
> > @@ -139,10 +139,13 @@
> >               return nil, errors.New("img not mounted")
> >       }
> >
> > -     return []string{
> > -             filepath.Join(img.Mountpoint, string(systemDataDir)),
> > -             filepath.Join(img.Mountpoint, string(systemDataDir2)),
> > -     }, nil
> > +     paths := []string{filepath.Join(img.Mountpoint,
> string(systemDataDir))}
> > +
> > +     if len(img.parts) == 3 {
> > +             paths = append(paths, filepath.Join(img.Mountpoint,
> string(systemDataDir2)))
> > +     }
> > +
> > +     return paths, nil
>
> This is part of another MR, right?
>

Yeah, this landed already even... just a prereq rendering issue


>
> >  }
> >
> >  //Mount the DiskImage
> >
> > === modified file 'ubuntu-device-flash/core.go'
> > --- ubuntu-device-flash/core.go       2014-11-25 19:31:44 +0000
> > +++ ubuntu-device-flash/core.go       2014-11-26 19:44:54 +0000
> > @@ -86,6 +86,8 @@
> >               return err
> >       }
> >
> > +     fmt.Println("Fetching information from server...")
> > +
> >       channels, err := ubuntuimage.NewChannels(globalArgs.Server)
> >       if err != nil {
> >               return err
> > @@ -108,6 +110,8 @@
> >       sigFilesChan := make(chan Files, len(sigFiles))
> >       defer close(sigFilesChan)
> >
> > +     fmt.Println("Downloading and setting up...")
> > +
> >       for _, f := range sigFiles {
> >               go bitDownloader(f, sigFilesChan, globalArgs.Server,
> cacheDir)
> >       }
> > @@ -121,7 +125,9 @@
> >       go func() {
> >               for i := 0; i < len(image.Files); i++ {
> >                       f := <-filesChan
> > -                     fmt.Println("Download finished for", f.FilePath)
> > +
> > +                     printOut("Download finished for", f.FilePath)
> > +
> >                       filePathChan <- f.FilePath
> >               }
> >               close(filePathChan)
> > @@ -143,7 +149,7 @@
> >               signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
> >
> >               for sig := range ch {
> > -                     fmt.Println("Received", sig, "... ignoring")
> > +                     printOut("Received", sig, "... ignoring")
> >               }
> >       }()
> >
> > @@ -268,6 +274,8 @@
> >       }
> >
> >       if coreCmd.DeveloperMode {
> > +             fmt.Println("Enabling developer mode...")
> > +
> >               authorizedKey, err := getAuthorizedSshKey()
> >               if err != nil {
> >                       return fmt.Errorf("failed to obtain a public key
> for developer mode: %s", err)
> > @@ -353,7 +361,7 @@
> >       if out, err := exec.Command("chroot", systemPath, "grub-install",
> "/root_dev").CombinedOutput(); err != nil {
> >               return fmt.Errorf("unable to install grub: %s", out)
> >       } else {
> > -             fmt.Println(string(out))
> > +             printOut(string(out))
> >       }
> >
> >       // ensure we run not into recordfail issue
> > @@ -376,7 +384,7 @@
> >       if out, err := exec.Command("chroot", systemPath,
> "update-grub").CombinedOutput(); err != nil {
> >               return fmt.Errorf("unable to update grub: %s", out)
> >       } else {
> > -             fmt.Println(string(out))
> > +             printOut(string(out))
> >       }
> >
> >       return nil
> >
> > === modified file 'ubuntu-device-flash/main.go'
> > --- ubuntu-device-flash/main.go       2014-11-04 13:46:00 +0000
> > +++ ubuntu-device-flash/main.go       2014-11-26 19:44:54 +0000
> > @@ -35,6 +35,7 @@
> >       Server        string `long:"server" description:"Use a different
> image server" default:"https://system-image.ubuntu.com"`
> >       CleanCache    bool   `long:"clean-cache" description:"Cleans up
> cache with all downloaded bits"`
> >       TLSSkipVerify bool   `long:"tls-skip-verify" description:"Skip TLS
> certificate validation"`
> > +     Verbose       bool   `long:"verbose" short:"v" description:"More
> messages will be printed out"`
> >  }
> >
> >  var globalArgs arguments
> > @@ -63,3 +64,9 @@
> >               os.Exit(1)
> >       }
> >  }
> > +
> > +func printOut(args ...interface{}) {
> > +     if globalArgs.Verbose {
> > +             fmt.Println(args...)
> > +     }
> > +}
> >
>
>
> --
>
> https://code.launchpad.net/~sergiusens/goget-ubuntu-touch/noverb/+merge/242965
> You are the owner of lp:~sergiusens/goget-ubuntu-touch/noverb.
>

-- 
https://code.launchpad.net/~sergiusens/goget-ubuntu-touch/noverb/+merge/242965
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~sergiusens/goget-ubuntu-touch/noverb into lp:goget-ubuntu-touch.



More information about the Ubuntu-reviews mailing list