Code review comment for lp:~linaro-infrastructure/cbuild/cbuild-scheduler_cbuild-lava

Revision history for this message
Stevan Radaković (stevanr) wrote :

> Only comment I have is for lines like these:
>
> 64 + - apt-get install -y wget
> 65 + - if grep -q Beagle /proc/cpuinfo; then
> 66 + - " # if it's supposed QEMU, download from local server"
> 67 + - url="http://10.0.2.2/images/tmp/gcc-
> linaro-4.6-2012.09-results.tar.gz"
> 68 + - else
> 69 + - url="http://people.linaro.org/~pfalcon/cbuild-lava/gcc-
> linaro-4.6-2012.09-results.tar.gz"
> 70 + - fi
>
> I see that there is a mix of syntax: some steps are quoted, some are not. It
> definitely will work on both cases, it is more from a consistency point of
> view. Was puzzled actually by how the yaml parser will handle that, but it
> handles them correctly anyway.

Yes, quoted and unquoted lines are parsed in the same way it seems. It may be logical to remove the quotes from that one line we have for consistency reasons.

>
> The other thing is including if-else bash structures in the yaml file: can we
> have a separate bash script and run that one? Or do we need to do other things
> before it is possible to achieve that: like sending the file to lava, etc. I
> know it's a couple of lines and is just the template... (oddily we might even
> move all the steps in a bash script and just execute that one).

I'm not sure if this can be done, but anyway I fail to see the difference between the current approach and the one you described. Only if we can extract some piece of shell code that would be used in multiple templates scripts would this make sense. But if it makes sense to try and play around with this approach we can certainly do so.

« Back to merge proposal