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

Issue 173803006: Split generate_global_constructors.py out of compute_dependencies.py (Closed)

Created:
6 years, 10 months ago by Nils Barth (inactive)
Modified:
6 years, 10 months ago
Reviewers:
haraken
CC:
blink-reviews, Nils Barth (inactive), kojih, arv+blink, jsbell+bindings_chromium.org, sof, abarth-chromium, marja+watch_chromium.org, adamk+blink_chromium.org, Nate Chapin, Inactive
Visibility:
Public.

Description

Split generate_global_constructors.py out of compute_dependencies.py (Long CL b/c refactoring a complex and messy script.) Currently compute_dependencies.py is very complex, doing 3 tasks: 1. Computing interfaces info (main function) 2. Computing global constructors 3. Computing event interfaces This splits out 2 (global constructors); event interfaces will be a followup. The complexity makes the code hard to understand and edit (previous CLs have required extensive review), and makes code reuse hard (e.g., in run-bindings-tests we need to use dummy files b/c we don't care about global constructors). Most seriously, this makes it hard to use compute_dependencies to compute dependencies at gyp time (needed to avoid full rebuilds), since it generates excess files that we only want to generate at build time. This also: * adds documentation, * renames compute_dependencies to compute_interfaces_info,  as it does a lot more than just compute dependencies now * makes the call to generate_global_constructors generic  (so we don't need to add a flag for each global scope!), and * factors out some common file-handling and regex IDL parsing functions. 2 followup CLs: * Split out EventInterfaces.in * Remove [NoInterfaceObject] from tests and dependencies: no longer needed, since we now just don't feed these into generate_global_constructors! BUG=341748 R=haraken

Patch Set 1 #

Patch Set 2 : Cleaner #

Unified diffs Side-by-side diffs Delta from patch set Stats (+425 lines, -880 lines) Patch
M Source/bindings/generated_bindings.gyp View 1 8 chunks +85 lines, -50 lines 0 comments Download
M Source/bindings/scripts/compute_dependencies.py View 1 1 chunk +0 lines, -557 lines 0 comments Download
A + Source/bindings/scripts/compute_interfaces_info.py View 1 16 chunks +75 lines, -261 lines 0 comments Download
A Source/bindings/scripts/generate_global_constructors.py View 1 1 chunk +135 lines, -0 lines 0 comments Download
A Source/bindings/scripts/utilities.py View 1 chunk +129 lines, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/bindings/main.py View 1 1 chunk +1 line, -12 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Nils Barth (inactive)
CL 1/2 of "factor compute_dependencies" (so we can use it in GYP). It's pretty long, ...
6 years, 10 months ago (2014-02-21 12:02:57 UTC) #1
Nils Barth (inactive)
Splitting up into 3 CLs, per discussion: 1. Rename file 2. Split (no logic changes) ...
6 years, 10 months ago (2014-02-21 12:32:00 UTC) #2
haraken
6 years, 10 months ago (2014-02-21 12:32:30 UTC) #3
On 2014/02/21 12:32:00, Nils Barth wrote:
> Splitting up into 3 CLs, per discussion:
> 1. Rename file
> 2. Split (no logic changes)
> 3. Logic changes (post-split cleanup)

Yeah, that is really helpful. Thanks!

Powered by Google App Engine
This is Rietveld 408576698