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

Issue 2325913002: recipe_modules/chromite: Use "build_type". (Closed)

Created:
4 years, 3 months ago by dnj
Modified:
4 years, 3 months ago
Reviewers:
Sergey Berezin, nxia1
CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org
Target Ref:
refs/heads/master
Project:
build
Visibility:
Public.

Description

recipe_modules/chromite: Use "build_type". Specialized config-specific parameters were previously being pushed from the master to the Chromite recipe, creating an information channel and dependency between the two. Chromite specifies all of the information that is needed in its configuration JSON, and that file is available to the recipe during execution. Load the build type from the configuration JSON instead of having it be pushed by the master. BUG=chromium:627996 TEST=expectations Committed: https://chromium.googlesource.com/chromium/tools/build/+/1575589a1d8f61fee45ec53ccbf77f073fd38847

Patch Set 1 #

Patch Set 2 : Keep variant API in for downstream compatibility. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+193 lines, -41 lines) Patch
M scripts/slave/recipe_modules/chromite/api.py View 1 4 chunks +56 lines, -4 lines 0 comments Download
M scripts/slave/recipe_modules/chromite/config.py View 1 5 chunks +30 lines, -8 lines 0 comments Download
M scripts/slave/recipe_modules/chromite/test_api.py View 2 chunks +10 lines, -3 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot.py View 1 7 chunks +44 lines, -9 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot.expected/chromiumos_coverage.json View 1 4 chunks +4 lines, -4 lines 0 comments Download
A + scripts/slave/recipes/cros/cbuildbot.expected/chromiumos_coverage_variant.json View 1 3 chunks +5 lines, -5 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot.expected/chromiumos_paladin.json View 1 1 chunk +18 lines, -0 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot.expected/chromiumos_paladin_manifest_failure.json View 1 1 chunk +18 lines, -0 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot_tryjob.py View 2 chunks +4 lines, -4 lines 0 comments Download
M scripts/slave/recipes/cros/cbuildbot_tryjob.expected/basic_compressed.json View 1 1 chunk +1 line, -1 line 0 comments Download
M scripts/slave/recipes/cros/cbuildbot_tryjob.expected/external.json View 1 1 chunk +1 line, -1 line 0 comments Download
M scripts/slave/recipes/cros/cbuildbot_tryjob.expected/internal.json View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (5 generated)
nxia1
dnj@, can we find someone to review this CL?
4 years, 3 months ago (2016-09-12 17:49:33 UTC) #2
dnj
PTAL
4 years, 3 months ago (2016-09-13 20:57:48 UTC) #4
Sergey Berezin
LGTM, though I can't claim I understood it all.
4 years, 3 months ago (2016-09-13 21:39:52 UTC) #5
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/2325913002/20001
4 years, 3 months ago (2016-09-15 15:28:30 UTC) #8
commit-bot: I haz the power
4 years, 3 months ago (2016-09-15 15:33:12 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/build/+/1575589a1d8f61fee45e...

Powered by Google App Engine
This is Rietveld 408576698