Merge lp://qastaging/~liuyq0307/linaro-android-build-tools/run-custom into lp://qastaging/linaro-android-build-tools

Proposed by Yongqin Liu
Status: Merged
Approved by: Paul Sokolovsky
Approved revision: 431
Merged at revision: 446
Proposed branch: lp://qastaging/~liuyq0307/linaro-android-build-tools/run-custom
Merge into: lp://qastaging/linaro-android-build-tools
Diff against target: 161 lines (+61/-21)
1 file modified
build-scripts/post-build-lava.py (+61/-21)
To merge this branch: bzr merge lp://qastaging/~liuyq0307/linaro-android-build-tools/run-custom
Reviewer Review Type Date Requested Status
Paul Sokolovsky Approve
Spring Zhang (community) Approve
Review via email: mp+101503@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-03-22.

Description of the change

add support for generating lava-job actions of running custom android command
the document about how to use is updated in https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration

To post a comment you must log in.
Revision history for this message
Paul Sokolovsky (pfalcon) wrote : Posted in a previous version of this proposal

Sorry that this takes long to review. One of the reason is that there're lot of "spurious" changes, not directly related to the implementation of requested feature. E.g. line 16 (and lot of others), where whitespace change made. I agree that keeping consistent and correct style is important matter, and those fixes look good, but they should be done separately from feature development (at the very least - in separate commits).

There's nothing can be done about that now, but please separate such changes in the future. Thanks.

Revision history for this message
Paul Sokolovsky (pfalcon) wrote : Posted in a previous version of this proposal

Now, another issue is that the changes appear to be big and complicated. Thanks much for making https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration - that helps a lot. But then it just proves that solution proposed is too complex UI-wise. One of the main requirements for android-build's build configs is that they are clean and simple, essentially one should be just able to look at it and immediately grasp what the build does.

The example as shown on https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration is very complicated, there's no way one can grasp what it is without studying docs and then carefully peering into that config for a while, nor it could be easily and safely changed - there's high probability of error. So, sorry, but this needs more work, and first of all, consideration of UI and syntax for such custom tests. Let me take it to the mailing list for discussion (need to ponder about it a bit first, to propose alternative).

review: Disapprove
Revision history for this message
Spring Zhang (qzhang) wrote :

I don't consider complicated issue, others can comment

review: Approve
Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

Looks good, thanks for rewriting it!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches