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

Issue 308743004: Move bindings_modules_generated from modules to bindings/modules (Closed)

Created:
6 years, 6 months ago by c.shu
Modified:
6 years, 6 months ago
CC:
blink-reviews, blink-reviews-bindings_chromium.org, arv+blink, abarth-chromium, (unused - use chromium), hjd, Peter Beverloo
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Move bindings_modules_generated from modules to bindings/modules There's a missing dependency b/c core_generated (webcore_generated) depends on modules_generated, due to partial interface definitions in modules extending classes in core. This will require significant work to fix (changing how we handle bindings for partial interface definitions), so for now move these files to bindings_modules_generated. This means we have a core -> bindings_modules dependency (which is allowed), instead of a core -> modules dependency (which is not allowed). This CL is a work-around until we split the files in bindings for core and modules eventually. BUG=378808 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175296

Patch Set 1 #

Patch Set 2 : Fix cyclic gyp dependency #

Total comments: 4

Patch Set 3 : Fix nits from review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+114 lines, -112 lines) Patch
M Source/bindings/modules/generated.gyp View 1 2 2 chunks +111 lines, -0 lines 0 comments Download
M Source/core/core.gyp View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M Source/modules/modules.gyp View 1 2 3 chunks +1 line, -111 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
c.shu
6 years, 6 months ago (2014-05-30 15:05:01 UTC) #1
eseidel
6 years, 6 months ago (2014-05-30 18:50:47 UTC) #2
Nils Barth (inactive)
LGTM with nits. https://codereview.chromium.org/308743004/diff/20001/Source/bindings/modules/generated.gyp File Source/bindings/modules/generated.gyp (right): https://codereview.chromium.org/308743004/diff/20001/Source/bindings/modules/generated.gyp#newcode22 Source/bindings/modules/generated.gyp:22: 'target_name': 'modules_bindings_generated', Could you rename this ...
6 years, 6 months ago (2014-06-02 02:02:09 UTC) #3
haraken
LGTM
6 years, 6 months ago (2014-06-02 02:19:06 UTC) #4
c.shu
https://codereview.chromium.org/308743004/diff/20001/Source/bindings/modules/generated.gyp File Source/bindings/modules/generated.gyp (right): https://codereview.chromium.org/308743004/diff/20001/Source/bindings/modules/generated.gyp#newcode22 Source/bindings/modules/generated.gyp:22: 'target_name': 'modules_bindings_generated', On 2014/06/02 02:02:09, Nils Barth wrote: > ...
6 years, 6 months ago (2014-06-02 15:48:16 UTC) #5
c.shu
The CQ bit was checked by c.shu@samsung.com
6 years, 6 months ago (2014-06-02 15:52:26 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/c.shu@samsung.com/308743004/40001
6 years, 6 months ago (2014-06-02 15:52:40 UTC) #7
commit-bot: I haz the power
6 years, 6 months ago (2014-06-02 17:04:01 UTC) #8
Message was sent while issue was closed.
Change committed as 175296

Powered by Google App Engine
This is Rietveld 408576698