Code review comment for lp:~doanac/ubuntu-test-cases/simplify-touch

Revision history for this message
Javier Collado (javier.collado) wrote :

The changes look good and the refactoring of the setup scripts was needed
indeed.

I have a few comments, but they're all related to issues that were already in
the code before this merge request, so I'm just writing them down here to see
what are your thoughts about them:

- Shell scripts arguments
While the scripts are documented and it's clear what environment variables have
to be set to use them, I'd prefer to have arguments, parse them and print the
usage string in case of failure.

- Spaces and tabs mixed
In jenkins.sh it seems that there are some lines that are indented using tabs
and other ones with spaces (and not always the same number of spaces). Being
python my main programming language, I'd prefer to use just 4 spaces per
indentation level.

- adb commands without serial number
Some adb commands don't have a serial number (one of them an adb push in this
merge proposal), neither had the old version of the script, so I guess it
works, but I feel like I'm mising something with regard to what device uses adb
by default.

review: Approve

« Back to merge proposal