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

Issue 2840563004: Move vr features.gni into its own directory. (Closed)

Created:
3 years, 8 months ago by mthiesse
Modified:
3 years, 8 months ago
Reviewers:
brettw, agrieve
CC:
chromium-reviews, jam, feature-vr-reviews_chromium.org, darin-cc_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move vr features.gni into its own directory. This fixes some weird issues we were encountering, where we had to hide some build targets behind (!is_ios) after adding our features target to ui/gfx. I'm not sure if any extra code was being compiled in, but asserts in gn files were being hit related to things that shouldn't be included on ios. It seems other features have already done this sort of thing. TBR=brettw@chromium.org BUG= Review-Url: https://codereview.chromium.org/2840563004 Cr-Commit-Position: refs/heads/master@{#467113} Committed: https://chromium.googlesource.com/chromium/src/+/faae24707ba8852ea92b56bcf77aa19878aacdab CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2840563004 Cr-Commit-Position: refs/heads/master@{#467167} Committed: https://chromium.googlesource.com/chromium/src/+/4cb289145e717dc3db6fe8073b9eab3d52a3b457

Patch Set 1 #

Patch Set 2 : Update references to vr/features.h #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -57 lines) Patch
M chrome/android/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/BUILD.gn View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/DEPS View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/about_flags.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/DEPS View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/chrome_jni_registrar.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/vr_shell/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/DEPS View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/features.gni View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M device/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M device/vr/BUILD.gn View 3 chunks +14 lines, -28 lines 0 comments Download
D device/vr/features.gni View 1 chunk +0 lines, -12 lines 0 comments Download
A device/vr/features/BUILD.gn View 1 chunk +15 lines, -0 lines 0 comments Download
A + device/vr/features/features.gni View 0 chunks +-1 lines, --1 lines 0 comments Download
M device/vr/vr_device_manager.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/gfx/DEPS View 1 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/font_render_params.h View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 27 (16 generated)
mthiesse
PTAL agrieve, I think with your review the rest can be TBR'd as mechanical changes.
3 years, 8 months ago (2017-04-25 18:48:40 UTC) #2
agrieve
lgtm
3 years, 8 months ago (2017-04-25 19:44:45 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/2840563004/1
3 years, 8 months ago (2017-04-25 21:03:52 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/faae24707ba8852ea92b56bcf77aa19878aacdab
3 years, 8 months ago (2017-04-25 21:10:27 UTC) #13
hcarmona
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2836013005/ by hcarmona@chromium.org. ...
3 years, 8 months ago (2017-04-25 21:28:37 UTC) #14
mthiesse
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2842513005/ by mthiesse@chromium.org. ...
3 years, 8 months ago (2017-04-25 21:29:34 UTC) #15
findit-for-me
Findit(https://goo.gl/kROfz5) confirmed this CL at revision 467113 as the culprit for failures in the build ...
3 years, 8 months ago (2017-04-25 21:40:13 UTC) #16
mthiesse
+TBR brettw@chromium.org
3 years, 8 months ago (2017-04-25 21:45:49 UTC) #20
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/2840563004/20001
3 years, 8 months ago (2017-04-25 21:46:29 UTC) #23
brettw
lgtm
3 years, 8 months ago (2017-04-25 22:43:57 UTC) #24
commit-bot: I haz the power
3 years, 8 months ago (2017-04-25 23:58:05 UTC) #27
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4cb289145e717dc3db6fe8073b9e...

Powered by Google App Engine
This is Rietveld 408576698