Bug 73540 - [EFL] Cleanup includes to reduce code complexity.
Summary: [EFL] Cleanup includes to reduce code complexity.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-01 00:27 PST by Ryuan Choi
Modified: 2011-12-01 22:39 PST (History)
5 users (show)

See Also:


Attachments
Patch (12.41 KB, patch)
2011-12-01 00:32 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.62 KB, patch)
2011-12-01 15:42 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (12.68 KB, patch)
2011-12-01 19:40 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2011-12-01 00:27:08 PST
EWebKit.h is a list of public header files for application to use WebKit/Efl.
For now, internal source files use it.

I'd like to cleanup includes in order to reduce code complexity,
Comment 1 Ryuan Choi 2011-12-01 00:32:55 PST
Created attachment 117368 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-12-01 05:08:40 PST
Comment on attachment 117368 [details]
Patch

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

The patch looks OK, but I'd like to have the issue below clarified before giving my informal r+.

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
> -#if ENABLE(SQL_DATABASE)
> -
> -#include "DatabaseDetails.h"
> -#include "DatabaseTracker.h"
> -#endif

I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?
Comment 3 Gustavo Noronha (kov) 2011-12-01 14:44:58 PST
Comment on attachment 117368 [details]
Patch

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

>> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
>> -#endif
> 
> I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?

I don't remember where I read this, but I do remember reading somewhere that having the headers included conditionally should be at the bottom, indeed.

> Source/WebKit/efl/ewk/ewk_frame.cpp:55
>  #include "ewk_private.h"
>  
>  #include <Eina.h>

This empty line should not exist.

> Source/WebKit/efl/ewk/ewk_private.h:39
> +#include "ewk_view.h"
>  
> -#include <cairo.h>
> +#include <Evas.h>

Same here.

> Source/WebKit/efl/ewk/ewk_window_features.cpp:28
>  #include "WindowFeatures.h"
> +#include "ewk_logging.h"
>  #include "ewk_private.h"
>  
>  #include <Eina.h>

Ditto.
Comment 4 Ryuan Choi 2011-12-01 15:42:36 PST
Created attachment 117506 [details]
Patch
Comment 5 Ryuan Choi 2011-12-01 15:53:38 PST
(In reply to comment #3)
> (From update of attachment 117368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117368&action=review
> 
> >> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
> >> -#endif
> > 
> > I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?
> 
> I don't remember where I read this, but I do remember reading somewhere that having the headers included conditionally should be at the bottom, indeed.
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:55
> >  #include "ewk_private.h"
> >  
> >  #include <Eina.h>
> 
> This empty line should not exist.
> 
> > Source/WebKit/efl/ewk/ewk_private.h:39
> > +#include "ewk_view.h"
> >  
> > -#include <cairo.h>
> > +#include <Evas.h>
> 
> Same here.
> 
> > Source/WebKit/efl/ewk/ewk_window_features.cpp:28
> >  #include "WindowFeatures.h"
> > +#include "ewk_logging.h"
> >  #include "ewk_private.h"
> >  
> >  #include <Eina.h>
> 
> Ditto.

Thanks for your review.

I fixed you mentioned.
Comment 6 WebKit Review Bot 2011-12-01 18:40:00 PST
Comment on attachment 117506 [details]
Patch

Rejecting attachment 117506 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
wk/ewk_settings.cpp
patching file Source/WebKit/efl/ewk/ewk_util.cpp
patching file Source/WebKit/efl/ewk/ewk_view.cpp
Hunk #2 FAILED at 48.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_view.cpp.rej
patching file Source/WebKit/efl/ewk/ewk_view_single.cpp
patching file Source/WebKit/efl/ewk/ewk_view_tiled.cpp
patching file Source/WebKit/efl/ewk/ewk_window_features.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/10725142
Comment 7 Ryuan Choi 2011-12-01 19:40:48 PST
Created attachment 117547 [details]
Patch
Comment 8 Ryuan Choi 2011-12-01 19:52:15 PST
Comment on attachment 117547 [details]
Patch

rebased.
Comment 9 WebKit Review Bot 2011-12-01 22:39:48 PST
Comment on attachment 117547 [details]
Patch

Clearing flags on attachment: 117547

Committed r101744: <http://trac.webkit.org/changeset/101744>
Comment 10 WebKit Review Bot 2011-12-01 22:39:53 PST
All reviewed patches have been landed.  Closing bug.