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

Issue 8764004: Add DEPS include rules so we don't accidentally use base (Closed)

Created:
9 years ago by dmichael (off chromium)
Modified:
9 years ago
Reviewers:
brettw
CC:
chromium-reviews, piman+watch_chromium.org, darin-cc_chromium.org, yzshen+watch_chromium.org, ihf+watch_chromium.org, viettrungluu
Visibility:
Public.

Description

Add DEPS include rules so we don't accidentally use chromium stuff. I basically turned off everything that is turned on in the root DEPS file, and then selectively turned stuff on. I had to provide some macros to replace the use of base/compilerspecific.h and build/buildconfig.h. I'm not sure if we have a coherent understanding of private vs trusted yet, but my thinking is that trusted can do anything, but private can't use trusted (it sounds like we may start selectively exposing some private interfaces to installed apps). BUG=None TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112512

Patch Set 1 #

Patch Set 2 : Refine include rules, fix violations. #

Patch Set 3 : Add forgotten DEPS files, fix use of WIN macro #

Total comments: 2

Patch Set 4 : Move OS-specific macros to ppapi/tests/test_utils.h #

Total comments: 1

Patch Set 5 : Remove rules to include self where possible #

Unified diffs Side-by-side diffs Delta from patch set Stats (+151 lines, -58 lines) Patch
A ppapi/c/DEPS View 1 1 chunk +11 lines, -0 lines 0 comments Download
A ppapi/cpp/DEPS View 1 1 chunk +16 lines, -0 lines 0 comments Download
M ppapi/cpp/file_io.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
A ppapi/cpp/private/DEPS View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
A ppapi/cpp/trusted/DEPS View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M ppapi/tests/DEPS View 1 1 chunk +13 lines, -0 lines 0 comments Download
M ppapi/tests/pp_thread.h View 1 2 3 4 chunks +9 lines, -14 lines 0 comments Download
M ppapi/tests/test_audio_config.cc View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
M ppapi/tests/test_file_io.cc View 1 3 chunks +10 lines, -11 lines 0 comments Download
M ppapi/tests/test_flash.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M ppapi/tests/test_net_address_private.cc View 1 2 3 2 chunks +5 lines, -6 lines 0 comments Download
M ppapi/tests/test_tcp_socket_private.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M ppapi/tests/test_transport.h View 1 3 chunks +9 lines, -4 lines 0 comments Download
M ppapi/tests/test_transport.cc View 1 6 chunks +16 lines, -11 lines 0 comments Download
M ppapi/tests/test_utils.h View 1 2 3 1 chunk +46 lines, -0 lines 0 comments Download
M ppapi/tests/test_var.cc View 1 2 chunks +1 line, -2 lines 0 comments Download
M ppapi/tests/test_var_deprecated.cc View 1 2 chunks +1 line, -2 lines 0 comments Download
M ppapi/tests/test_websocket.cc View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
dmichael (off chromium)
9 years ago (2011-12-01 06:32:13 UTC) #1
brettw
http://codereview.chromium.org/8764004/diff/2034/ppapi/api/pp_macros.idl File ppapi/api/pp_macros.idl (right): http://codereview.chromium.org/8764004/diff/2034/ppapi/api/pp_macros.idl#newcode23 ppapi/api/pp_macros.idl:23: #if defined(__APPLE__) It doesn't seem like all this code ...
9 years ago (2011-12-01 18:29:22 UTC) #2
dmichael (off chromium)
http://codereview.chromium.org/8764004/diff/2034/ppapi/api/pp_macros.idl File ppapi/api/pp_macros.idl (right): http://codereview.chromium.org/8764004/diff/2034/ppapi/api/pp_macros.idl#newcode23 ppapi/api/pp_macros.idl:23: #if defined(__APPLE__) On 2011/12/01 18:29:22, brettw wrote: > It ...
9 years ago (2011-12-01 18:40:53 UTC) #3
brettw
lgtm http://codereview.chromium.org/8764004/diff/6002/ppapi/cpp/trusted/DEPS File ppapi/cpp/trusted/DEPS (right): http://codereview.chromium.org/8764004/diff/6002/ppapi/cpp/trusted/DEPS#newcode3 ppapi/cpp/trusted/DEPS:3: "+ppapi/c/trusted", It shouldn't be necessary to include the ...
9 years ago (2011-12-01 18:57:02 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmichael@chromium.org/8764004/11004
9 years ago (2011-12-01 19:04:06 UTC) #5
commit-bot: I haz the power
9 years ago (2011-12-01 20:12:39 UTC) #6
Change committed as 112512

Powered by Google App Engine
This is Rietveld 408576698