Bug 161854

Summary: [CMake] Refactor GENERATE_BINDINGS
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, clopez, commit-queue, gyuyoung.kim, lforschler
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 161433    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

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.