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

Revision history for this message
Milo Casagrande (milo) 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.

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).

« Back to merge proposal