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

[Sergio Schvezov]

On viernes 27 de marzo de 2015 11h'46:10 ART, Michael Vogt wrote:
> Review: Approve
>
> Thanks, looks good, I have some inline comments, but nothing relevant.
>
> Diff comments:
>
>> === modified file 'diskimage/common.go'
>> --- diskimage/common.go	2015-01-28 19:37:00 +0000
>> +++ diskimage/common.go	2015-03-27 13:26:46 +0000
>> @@ -53,7 +53,7 @@
>>  type CoreImage interface { ...
>
> This could be implemented as a map unless I'm mistaken:
> const archDeviceMapping map[string]string{"armhf": 
> "generic_armhf", "amd64": "generic_amd64", } which would be 
> slightly compacter. Not really important though and I just 
> noticed that the consts are used below for other purposes 
> so…nevermind.

I thought about this, but I need to use it elsewhere, that said, I can use 
the snappy definitions now too.

>> +	archArmhf = "armhf"
>> +	archAmd64 = "amd64"
>> +	archi386  = "i386"
>> +)
>> + ...
>
> Just curious, does this really matter? Or could "packageQueue 
> := []string{nil}" and you just grow it via the append()? Or is 
> that a much less efficient operation (i.e. lots of reallocs)? 
> Its fine, I'm just curious.

Since this is a small dataset it might not matter that much, but I don't 
want to lose the practice when I deal with pre known sizes, each append 
that surpasses the capacity means a relocation, and look at the defaults:

http://play.golang.org/p/QtTmxvNAzU

More here: http://blog.golang.org/go-slices-usage-and-internals

>
>> +
>> +	if coreCmd.Oem != "" {
>> +		packageQueue = append(packageQueue, coreCmd.Oem)
>> +	}
>> +	for k := range coreCmd.oem.Config {
>> +		// ubuntu-core is not a real package
>> +		if k != "ubuntu-core" {
>
> What happens here? Is snappy just returning a error or is the 
> world exploding? If the world explodes we might want to fix 
> snappy too :)

The world explodes (panic I pastebined to you)

>
>> +			packageQueue = append(packageQueue, k)
>> +		}
>> +	}
>> +	packageQueue = append(packageQueue, coreCmd.Install...)
>> + ...
>
> Heh, clever!

Thanks :-)

>> +	defer snappy.SetRootDir("/")
>> +
>> +	flags := snappy.InhibitHooks
>> +	if coreCmd.Development.DeveloperMode {
>> +		flags |= snappy.AllowUnauthenticated ...
>
>


-- 
https://code.launchpad.net/~sergiusens/goget-ubuntu-touch/readOEMPackageYaml/+merge/254392
Your team Ubuntu Phablet Team is subscribed to branch lp:goget-ubuntu-touch.