Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(18)

Issue 2415963004: Fix compile on the Blimp Client Engine Integration bot (Closed)

Created:
4 years, 2 months ago by shenghuazhang
Modified:
4 years, 2 months ago
CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org
Target Ref:
refs/heads/master
Project:
build
Visibility:
Public.

Description

Fix compile on the Blimp Client Engine Integration bot BUG=655864 Committed: https://chromium.googlesource.com/chromium/tools/build/+/61728af22d26f33d09ed715d0c43f6051ee9c1ea

Patch Set 1 #

Total comments: 6

Patch Set 2 : nit #

Patch Set 3 : call chromium.set_config first #

Total comments: 4

Patch Set 4 : Case comment #

Total comments: 2

Patch Set 5 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -360 lines) Patch
M scripts/slave/recipes/blimp/integration.py View 1 2 3 4 4 chunks +7 lines, -4 lines 0 comments Download
M scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json View 1 2 3 1 chunk +0 lines, -350 lines 0 comments Download
A + scripts/slave/recipes/blimp/integration.expected/Blimp_Client_Engine_Integration_test_pass.json View 1 2 3 5 chunks +9 lines, -6 lines 0 comments Download

Messages

Total messages: 18 (4 generated)
shenghuazhang
4 years, 2 months ago (2016-10-14 01:12:10 UTC) #2
mikecase (-- gone --)
https://codereview.chromium.org/2415963004/diff/1/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/1/scripts/slave/recipes/blimp/integration.py#newcode42 scripts/slave/recipes/blimp/integration.py:42: android_build = api.path['checkout'].join('src', 'out-android') need to change these paths ...
4 years, 2 months ago (2016-10-14 02:25:36 UTC) #3
shenghuazhang
Switch calling sequence for chromium.set_config and api.chromium_android.configure_from_properties. https://codereview.chromium.org/2415963004/diff/1/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/1/scripts/slave/recipes/blimp/integration.py#newcode42 scripts/slave/recipes/blimp/integration.py:42: android_build = ...
4 years, 2 months ago (2016-10-14 03:21:19 UTC) #4
shenghuazhang
https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json File scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json (right): https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json#newcode217 scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json:217: "[SLAVE_BUILD]/src/out-linux" Wait... it causes more changes like this. Probably ...
4 years, 2 months ago (2016-10-14 03:25:32 UTC) #5
shenghuazhang
https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json File scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json (right): https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json#newcode217 scripts/slave/recipes/blimp/integration.expected/Blimp_Android_Tester_test_pass.json:217: "[SLAVE_BUILD]/src/out-linux" On 2016/10/14 03:25:32, shenghuazhang1 wrote: > Wait... it ...
4 years, 2 months ago (2016-10-14 03:37:10 UTC) #6
mikecase (-- gone --)
https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.py#newcode64 scripts/slave/recipes/blimp/integration.py:64: build_dir=linux_build, You should probably add the "Debug" back here. ...
4 years, 2 months ago (2016-10-14 16:48:34 UTC) #7
shenghuazhang
https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/40001/scripts/slave/recipes/blimp/integration.py#newcode64 scripts/slave/recipes/blimp/integration.py:64: build_dir=linux_build, On 2016/10/14 16:48:34, mikecase wrote: > You should ...
4 years, 2 months ago (2016-10-14 18:32:25 UTC) #8
mikecase (-- gone --)
looks good. Just 1 nit to fix. https://codereview.chromium.org/2415963004/diff/60001/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/60001/scripts/slave/recipes/blimp/integration.py#newcode65 scripts/slave/recipes/blimp/integration.py:65: api.chromium.c.build_config_fs) Indenting ...
4 years, 2 months ago (2016-10-14 19:04:14 UTC) #9
shenghuazhang
https://codereview.chromium.org/2415963004/diff/60001/scripts/slave/recipes/blimp/integration.py File scripts/slave/recipes/blimp/integration.py (right): https://codereview.chromium.org/2415963004/diff/60001/scripts/slave/recipes/blimp/integration.py#newcode65 scripts/slave/recipes/blimp/integration.py:65: api.chromium.c.build_config_fs) On 2016/10/14 19:04:14, mikecase wrote: > Indenting should ...
4 years, 2 months ago (2016-10-14 19:27:23 UTC) #10
shenghuazhang
+Dirk Can you please have a review on the fix?
4 years, 2 months ago (2016-10-14 19:54:01 UTC) #12
mikecase (-- gone --)
lgtm
4 years, 2 months ago (2016-10-14 20:05:18 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415963004/80001
4 years, 2 months ago (2016-10-14 20:13:27 UTC) #15
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/tools/build/+/61728af22d26f33d09ed715d0c43f6051ee9c1ea
4 years, 2 months ago (2016-10-14 20:17:48 UTC) #17
Dirk Pranke
4 years, 2 months ago (2016-10-15 17:23:34 UTC) #18
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698