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

Issue 1913703002: Add support for cros chrome_sdk (simplechrome) to MB. (Closed)

Created:
4 years, 8 months ago by Dirk Pranke
Modified:
4 years, 7 months ago
Reviewers:
stevenjb
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@mb_check_gen
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add support for cros chrome_sdk (simplechrome) to MB. When we build Chrome for ChromeOS using the simplechrome workflow, all of the GYP_DEFINES and gn args that are needed are determined by the cros wrapper scripts, and rather than duplicate that logic in the MB mixins, it's easier to just pass the data through. However, we don't want this passthrough mechanism to be abused, so we implement so that this only works if the bot is configured to allow for passthrough *and* the GYP_DEFINES or GN_ARGS env vars contain chromeos=1 or target_os="chromeos" as appropriate. If we need to be less strict later we can alway change things. R=stevenjb@chromium.org BUG=605154 Committed: https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d Cr-Commit-Position: refs/heads/master@{#389532}

Patch Set 1 : initial concept for review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -9 lines) Patch
M tools/mb/mb.py View 7 chunks +39 lines, -8 lines 0 comments Download
M tools/mb/mb_config.pyl View 2 chunks +12 lines, -1 line 0 comments Download

Messages

Total messages: 10 (4 generated)
Dirk Pranke
4 years, 8 months ago (2016-04-23 02:33:29 UTC) #2
stevenjb
This lgtm, but if there is someone else familiar with MB or simple chrome available ...
4 years, 7 months ago (2016-04-25 17:56:00 UTC) #4
Dirk Pranke
I'm not too worried about this; the changes are trivial.
4 years, 7 months ago (2016-04-25 18:13:38 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913703002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913703002/40001
4 years, 7 months ago (2016-04-25 18:14:09 UTC) #7
commit-bot: I haz the power
Committed patchset #1 (id:40001)
4 years, 7 months ago (2016-04-25 19:18:43 UTC) #8
commit-bot: I haz the power
4 years, 7 months ago (2016-04-25 19:20:23 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d
Cr-Commit-Position: refs/heads/master@{#389532}

Powered by Google App Engine
This is Rietveld 408576698