Bug 96797 – [WK2] Order includes according to style guide

• Status: NEW
• Component: WebKit2

[Kenneth Rohde Christiansen]

Let's order the includes with my little script. Checked with check-webkit-style.

[Kenneth Rohde Christiansen]

Created attachment 164167 [details]
Patch

[Kenneth Rohde Christiansen]

Created attachment 164172 [details]
Script

If anyone knows python better than me, this script can probably be fixed up and included in Tools/Scripts

[Kenneth Rohde Christiansen]

The script can be used like this:

find Source/WebKit2 -name '*' | xargs reorder-includes.py

[Chris Dumez]

Comment on attachment 164167 [details]
Patch

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

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:27
> +#import <wtf/Platform.h>

I believe this one should be after the blank line.

> Source/WebKit2/UIProcess/API/qt/qwebkittest_p.h:25
> +#include "qwebkitglobal.h"

I'm not familiar with Qt port but should "qwebkitglobal.h" always be first? like "config.h"?

[Kenneth Rohde Christiansen]

(In reply to comment #4)
> (From update of attachment 164167 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164167&action=review
> 
> > Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:27
> > +#import <wtf/Platform.h>
> 
> I believe this one should be after the blank line.
> 
> > Source/WebKit2/UIProcess/API/qt/qwebkittest_p.h:25
> > +#include "qwebkitglobal.h"
> 
> I'm not familiar with Qt port but should "qwebkitglobal.h" always be first? like "config.h"?

Yeah it should. I can make an exception for that in the script

[Kenneth Rohde Christiansen]

Created attachment 164173 [details]
Patch

[WebKit Review Bot]

Attachment 164173 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/UIProcess/API/qt/qquicknetworkreply_p.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 84 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

LGTM.

[Early Warning System Bot]

Comment on attachment 164173 [details]
Patch

Attachment 164173 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13850610

[Kenneth Rohde Christiansen]

Seems that some have issues with includes depending on order. Simon, can you have a look at Qt?

[Simon Hausmann]

Comment on attachment 164173 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquicknetworkreply_p.h:24
> +#include "qwebkitglobal.h"

qwebkitglobal.h provides us with the QWEBKIT_EXPORT macro. All that we need is that it is defined before the class declaration starts, but otherwise it can be included in any order. Feel free to shuffle it around to make check-webkit-style happy :)

[Benjamin Poulain]

Comment on attachment 164173 [details]
Patch

This likely needs a rebaseline. And it should make the style bot happy.

Ping someone when you update so that it does not stay in queue forever.