Bug 161854 - [CMake] Refactor GENERATE_BINDINGS
Summary: [CMake] Refactor GENERATE_BINDINGS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 161433
  Show dependency tree / graph
 
Reported: 2016-09-11 20:10 PDT by Fujii Hironori
Modified: 2016-09-15 10:50 PDT (History)
6 users (show)

See Also:

Attachments
Patch (17.72 KB, patch)
2016-09-11 20:30 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (17.54 KB, patch)
2016-09-11 21:46 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (17.26 KB, patch)
2016-09-12 01:10 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (17.25 KB, patch)
2016-09-12 02:53 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2016-09-11 20:10:24 PDT 
[CMake] Refator GENERATE_BINDINGS

I want to do some refatoring on GENERATE_BINDINGS for the preparation of Bug 161433.

* Use CMakeParseArguments for argument parsing
* Use function instread of macro for its own variable scope
* Wrap both preprocess-idls.pl and generate-bindings.pl scripts
* Donwcase a local variable COMMON_GENERATOR_DEPENDENCIES
Comment 1 Fujii Hironori 2016-09-11 20:30:23 PDT 
Created attachment 288546 [details]
Patch
Comment 2 Fujii Hironori 2016-09-11 21:46:23 PDT 
Created attachment 288548 [details]
Patch
Comment 3 Alex Christensen 2016-09-12 00:01:55 PDT 
Comment on attachment 288548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288548&action=review

> Tools/DumpRenderTree/CMakeLists.txt:129
> +    PREFIX JS

Could we get rid of this now that the only generated bindings are JS?
Comment 4 Fujii Hironori 2016-09-12 00:47:49 PDT 
Thank you for reviewing my patch.

(In reply to comment #3)
> Could we get rid of this now that the only generated bindings are JS?

Yes. I'll remove PREFIX and EXTENSION.
Comment 5 Gyuyoung Kim 2016-09-12 00:48:54 PDT 
Comment on attachment 288548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288548&action=review

> Source/WebCore/CMakeLists.txt:3626
> +    GENERATOR JS

One question, will you use other generator(e.g. v8) in future ?
Comment 6 Fujii Hironori 2016-09-12 01:10:02 PDT 
Created attachment 288558 [details]
Patch
Comment 7 Fujii Hironori 2016-09-12 01:18:19 PDT 
(In reply to comment #5)
> > Source/WebCore/CMakeLists.txt:3626
> > +    GENERATOR JS
> 
> One question, will you use other generator(e.g. v8) in future ?

No, I don't.
This 'GENERATOR' argument takes 'JS', 'TestRunner' or 'DumpRenderTree' now.
Comment 8 Fujii Hironori 2016-09-12 02:53:18 PDT 
Created attachment 288562 [details]
Patch

Fix typos.
Comment 9 Gyuyoung Kim 2016-09-12 17:37:38 PDT 
Comment on attachment 288562 [details]
Patch

Patch looks good to me. r=me. However it would be good if someone might to want to have final review before landing.
Comment 10 Fujii Hironori 2016-09-15 03:14:07 PDT 
Thank you for r+. Could anyone give cq+?
Comment 11 WebKit Commit Bot 2016-09-15 10:50:13 PDT 
Comment on attachment 288562 [details]
Patch

Clearing flags on attachment: 288562

Committed r205982: <http://trac.webkit.org/changeset/205982>
Comment 12 WebKit Commit Bot 2016-09-15 10:50:19 PDT 
All reviewed patches have been landed.  Closing bug.