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

Proposed by Yongqin Liu
Status: Superseded
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 Disapprove
Linaro Validation Team Pending
Review via email: mp+98825@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2012-04-11.

Description of the change

add support to generate run-custom actions for lava-job

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

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 :

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
430. By Yongqin Liu

modify to support only android command from android-build page

431. By Yongqin Liu

sort the test variable names

Unmerged revisions

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