Bug 101077

Summary: [GTK] Clean up includes in GObject DOM bindings code
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric.carlson, feature-media-reviews, haraken, japhet, webkit.review.bot, xan.lopez, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 101074    
Bug Blocks: 101079    
Attachments:
Description Flags
Patch
haraken: review+
Updated patch to include test results haraken: review+

Description Carlos Garcia Campos 2012-11-02 11:35:38 PDT 
There are some headers included multiple times and some inconsistencies in the includes.
Comment 1 Carlos Garcia Campos 2012-11-02 11:38:05 PDT 
Created attachment 172095 [details]
Patch
Comment 2 Kentaro Hara 2012-11-02 11:58:00 PDT 
Comment on attachment 172095 [details]
Patch

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

Looks OK. Let's land it after making EWS bots green.

> Source/WebCore/ChangeLog:24
> +        * bindings/scripts/CodeGeneratorGObject.pm:

No need to update run-bindings-tests?
Comment 3 Carlos Garcia Campos 2012-11-02 12:03:54 PDT 
(In reply to comment #2)
> (From update of attachment 172095 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172095&action=review
> 
> Looks OK. Let's land it after making EWS bots green.
> 
> > Source/WebCore/ChangeLog:24
> > +        * bindings/scripts/CodeGeneratorGObject.pm:
> 
> No need to update run-bindings-tests?

Yes, I dind't know there were tests for the bindings :-P I'm updating the results for all the patches now. Thanks!
Comment 4 Kentaro Hara 2012-11-02 12:07:29 PDT 
(In reply to comment #3)
> Yes, I dind't know there were tests for the bindings :-P I'm updating the results for all the patches now. Thanks!

See here: https://trac.webkit.org/wiki/WebKitIDL#RunBindingsTests
Comment 5 Carlos Garcia Campos 2012-11-02 12:18:20 PDT 
Created attachment 172106 [details]
Updated patch to include test results
Comment 6 Martin Robinson 2012-11-02 13:15:23 PDT 
Comment on attachment 172106 [details]
Updated patch to include test results

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

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1437
>      # Remove the implementation header from the list of included files.

It looks like this comment can be removed now?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1438
>      %includesCopy = %implIncludes;

Maybe we don't have to make a copy if we don't do the deletion?
Comment 7 Carlos Garcia Campos 2012-11-08 06:35:23 PST 
Committed r133893: <http://trac.webkit.org/changeset/133893>