[Findit] pass compile_targets using build_parameter(recipe side change).

[Findit] pass compile_targets using build_parameter(recipe side change).

BUG=621266
Committed: https://chromium.googlesource.com/chromium/tools/build/+/d4709d9d4335cbc59ffe73a659276646ab19c1bc

[chanli, 2016-06-20 23:26:02]

chanli@chromium.org changed reviewers:
+ lijeffrey@chromium.org, stgao@chromium.org

[chanli, 2016-06-20 23:26:02]

ptal

[lijeffrey, 2016-06-20 23:42:18]

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:43: help='The buildbucket
property in which we can find build id.'
nit: when printed, will this have a space between "build id." and "We need"?
i.e. "...build id.We need..."

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:126: # compile_targets should
be saved in build parameter in this case.
nit: build parameters in this case.

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:127: buildbucket_json =
json.loads(buildbucket)
is possible for buildbucket to be None? If this is the case an exception will be
raised

[chanli, 2016-06-21 00:52:33]

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:43: help='The buildbucket
property in which we can find build id.'
On 2016/06/20 23:42:18, lijeffrey wrote:
> nit: when printed, will this have a space between "build id." and "We need"?
> i.e. "...build id.We need..."

Done.

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:126: # compile_targets should
be saved in build parameter in this case.
On 2016/06/20 23:42:18, lijeffrey wrote:
> nit: build parameters in this case.

It's actually build_parameter..

https://codereview.chromium.org/2088593002/diff/20001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:127: buildbucket_json =
json.loads(buildbucket)
On 2016/06/20 23:42:18, lijeffrey wrote:
> is possible for buildbucket to be None? If this is the case an exception will
be
> raised

I don't think buildbucket would be None.

[lijeffrey, 2016-06-21 01:03:54]

lgtm

[stgao, 2016-06-21 05:43:16]

lgtm with a nit.

https://codereview.chromium.org/2088593002/diff/40001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/2088593002/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:126: # compile_targets should
be saved in build parameter in this case.
nit: It might not be set though if no ninja output nodes are extracted.

[chanli, 2016-06-21 21:51:19]

https://codereview.chromium.org/2088593002/diff/40001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/2088593002/diff/40001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:126: # compile_targets should
be saved in build parameter in this case.
On 2016/06/21 05:43:16, stgao wrote:
> nit: It might not be set though if no ninja output nodes are extracted.

Done.

[chanli, 2016-06-21 21:51:39]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-06-21 21:51:39]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org,
lijeffrey@chromium.org
Link to the patchset: https://codereview.chromium.org/2088593002/#ps60001
(title: ".")

[commit-bot: I haz the power, 2016-06-21 21:51:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2088593002/60001

[commit-bot: I haz the power, 2016-06-21 21:55:45]

Description was changed from

==========
[Findit] pass compile_targets using build_parameter(recipe side change).

BUG=621266
==========

to

==========
[Findit] pass compile_targets using build_parameter(recipe side change).

BUG=621266

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/d4709d9d4335cbc59ffe...
==========

[commit-bot: I haz the power, 2016-06-21 21:55:47]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/tools/build/+/d4709d9d4335cbc59ffe...