Code review comment for lp://qastaging/~deeptik/linaro-ci/fix-bug1013611

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

On Mon, Jun 18, 2012 at 2:47 PM, Milo Casagrande
<email address hidden>wrote:

> Review: Needs Fixing
>
> Hello Deepti,
>
> thanks for working on this!
> Merge proposal looks good to me, the only thing I will add is or a README
> file with a little bit of docs/description of the various scripts, or small
> comments per script that describe the script intents (even if they are not
> that complex, at least it will be easier for other to pick up faster what
> has been done).
>
> Thanks for the quick review.
I included lines in each of the scripts as its more easy to understand the
usage of the scripts.

> Also, I see that two files have license headers, others do not: what is
> the policy in these cases? Should all source code files have a license
> header?
>

The license header is for 2011 I will remove it for now, and would speak
with Danilo and the whole team on what is the appropriate header to be
included in the scripts.

>
> Another thing, but definitely minor: files use underscores or dashes in
> their names. There are file names with the latter and other with the
> former. Should we use just one?
>

AFAIK, there is no particular naming convention that I came across in
various tools in infrastructure.

>
> --
> https://code.launchpad.net/~deeptik/linaro-ci/fix-bug1013611/+merge/110740<https://code.launchpad.net/%7Edeeptik/linaro-ci/fix-bug1013611/+merge/110740>
> You are the owner of lp:~deeptik/linaro-ci/fix-bug1013611.
>

--
Thanks and Regards,
Deepti

« Back to merge proposal