Merge lp://staging/~liuyq0307/linaro-android-build-tools/add-monkey-l-a-t into lp://staging/linaro-android-build-tools

Hi,

This looks great, thanks, and there are some good cleanups
here too.

Is the new linaro_android_test_run action available on validation.linaro.org
and able to run both monkey and 0xbench?

If not then we will have to hold off on deploying this until it is.

54 + # Board-specific parameters

Extra space included in the indentation on that line.

122 + submin_results_command = "submit_results"
123 + if image_type == 'android':
124 + submin_results_command = "submit_results_on_host"

Typo in variable name, it should be "submit_results_command"

Thanks,

James

+ image_type = os.environ.get("IMAGE_TYPE")

What's being done here? If it calls for adding another build config variable, it's likely superfluous (unless there're reasons why it's not).

Also, braces placement needs fixing, it for sure worse than was before. Rule of thumb is to just follow style of the existing code (and minimize any whitespace changes).

And can you please look thru associated blueprint's whiteboard, it seems to have typos which make it a bit unclear ("Acceptance: Monkey for as long as it can", "Update the meeting to a weekly")

thanks for review
> Is the new linaro_android_test_run action available on validation.linaro.org
> and able to run both monkey and 0xbench?
yes, linaro_android_test_run is available on validation.linaro.org, and can run both monkey and 0xbench

> If not then we will have to hold off on deploying this until it is.
>
> 54 + # Board-specific parameters
>
> Extra space included in the indentation on that line.
sorry, delete the extra space

> 122 + submin_results_command = "submit_results"
> 123 + if image_type == 'android':
> 124 + submin_results_command = "submit_results_on_host"
>
> Typo in variable name, it should be "submit_results_command"
sorry, I will fixing it.

> + image_type = os.environ.get("IMAGE_TYPE")
>
> What's being done here? If it calls for adding another build config variable,
> it's likely superfluous (unless there're reasons why it's not).
sorry, I didn't recognize that this file only be used for android-build.
I will delete the relevant source.

> Also, braces placement needs fixing, it for sure worse than was before. Rule
> of thumb is to just follow style of the existing code (and minimize any
> whitespace changes).
I have modify the braces placement according the policy that minimize any whitespace changes.
BTW, is there any coding style for braces?

> And can you please look thru associated blueprint's whiteboard, it seems to
> have typos which make it a bit unclear ("Acceptance: Monkey for as long as it
> can", "Update the meeting to a weekly")
I have updated the bp, is it ok now.

And is there any environment that I can test this file?
I want to see after the execution of this file, is the result displayed ok.

On Mon, 26 Sep 2011 08:07:25 -0000
Yongqin Liu <email address hidden> wrote:

> > + image_type = os.environ.get("IMAGE_TYPE")
> >
> > What's being done here? If it calls for adding another build config
> > variable, it's likely superfluous (unless there're reasons why it's
> > not).
> sorry, I didn't recognize that this file only be used for
> android-build. I will delete the relevant source.

Thanks.

>
> > Also, braces placement needs fixing, it for sure worse than was
> > before. Rule of thumb is to just follow style of the existing code
> > (and minimize any whitespace changes).
> I have modify the braces placement according the policy that minimize
> any whitespace changes. BTW, is there any coding style for braces?

Well, we generally follow PEP8, though some grounded deviation may be
used. By general rule is the same as for C: everyone places braces in
different way, but when you submit patch for some code, you're expected
to follow the style of surrounding code. If you really think that
formatting should be changed, it should be separate commit - just
change formatting, no semantic changes.

> > And can you please look thru associated blueprint's whiteboard, it
> > seems to have typos which make it a bit unclear ("Acceptance:
> > Monkey for as long as it can", "Update the meeting to a weekly")
> I have updated the bp, is it ok now.
>
> And is there any environment that I can test this file?
> I want to see after the execution of this file, is the result
> displayed ok.

Thanks for bringing that up, that's how we'd like to handle such
changes for sure - test them on a sandbox before deploying on
production. I've created one at
https://ec2-75-101-195-88.compute-1.amazonaws.com/

Reviewing changes once again:

97 - "server": "http://validation.linaro.org/lava-server/dashboard",
98 + "server": "http://validation.linaro.org/lava-server/RPC2/",

Are you sure this is right? Why exactly such change is needed?

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

thanks for review.

Thanks for bringing that up, that's how we'd like to handle such
> changes for sure - test them on a sandbox before deploying on
> production. I've created one at
> https://ec2-75-101-195-88.compute-1.amazonaws.com/
>
Thanks, but until now there hasn't been one build:(
How can i make a build there?

> Reviewing changes once again:
>
> 97 - "server": "http://validation.linaro.org/lava-server/dashboard",
> 98 + "server": "http://validation.linaro.org/lava-server/RPC2/",
>
> Are you sure this is right? Why exactly such change is needed?
>

yes, this is right. the dashoard one is deprecated.
and in the future we will use the new url, see the hlep of api about
lava-server.
http://validation.linaro.org/lava-server/api/help/

Thanks,
Yongqin Liu

Thanks for clarification and checking that the sandbox now works. So, please test that it works as expected, and I'll merge it then.