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

Revision history for this message
Andy Doan (doanac) wrote :

On 08/23/2013 05:58 AM, Javier Collado wrote:
> Review: Approve

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

That's a good point. Given all the changes dependent on this change, I
think we should do this in as a follow-on. It would make things nicer
though.

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

I converted to tabs. I hate spaces. Guido is a genius for Python but
evil for spaces :)

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

As Paul said, we should probably just remove the -s $ANDROID_SERIAL as a
follow-on.

« Back to merge proposal