Code review comment for lp:~le-chi-thu/linaro-ci/add-testplan-support

Revision history for this message
Deepti B. Kalakeri (deeptik) wrote :

Thanks for addressing the review, I have few more.

1) Please retain the boot_linaro_image stuff as it was before you made the
changes, since this block does not change, I would like to have it in the
dict itself.

2) I did not see the 80 columns rule being followed, for ex line no: 99
spans to 100 columns.

3) I guess you did not see the following comments in the previous review


The regex above should not match first part(:100) of the following
LAVA_TEST_PLAN=":100, test_bcd".
The above regex matches :100 as testcase which is invalid.


> +
> + if"options"):
> + lava_test_run_command["parameters"]["test_options"] =
> +
> + template['actions'].append(lava_test_run_command)
> +

It would be good if combined both the for loops into one for loop.

could you please address all of them and resubmit.

On Tue, May 8, 2012 at 5:53 PM, Le Chi Thu <email address hidden> wrote:

> Review: Resubmit
> ltp take options - see the $OPTIONS macro in the test definition file.
> RUNSTEPS = ['cd build && sudo ./runltp $(OPTIONS)']
> Now the json file is correct generate.
> --
> You are reviewing the proposed merge of
> lp:~le-chi-thu/linaro-ci/add-testplan-support into lp:linaro-ci.

Thanks and Regards,
Infrastructure Team Member, Linaro Platform Teams | Open source software for ARM SoCs
Follow Linaro:!/linaroorg -

« Back to merge proposal