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

Ricardo Salveti rsalveti at rsalveti.net
Fri Nov 28 05:03:41 UTC 2014


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?

>  }
>  
>  //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
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