Bug 16676 - Apply GTK coding style to WebKit Gtk public headers
Summary: Apply GTK coding style to WebKit Gtk public headers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2007-12-30 05:38 PST by Michael Natterer
Modified: 2008-06-10 01:08 PDT (History)
2 users (show)

See Also:


Attachments
Apply GTK coding style (121.59 KB, patch)
2007-12-30 06:08 PST, Michael Natterer
alp: review-
Details | Formatted Diff | Diff
Apply gtk+ style to headers (26.88 KB, patch)
2008-06-07 13:43 PDT, Christian Dywan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Natterer 2007-12-30 05:38:14 PST
Summary says it all :)
Comment 1 Michael Natterer 2007-12-30 06:08:25 PST
Created attachment 18191 [details]
Apply GTK coding style
Comment 2 Darin Adler 2007-12-30 22:35:53 PST
Comment on attachment 18191 [details]
Apply GTK coding style

Seems smart to apply GTK coding style to the headers. But seems strange to do it inside the files. I'll let the GTK WebKit hackers make the call, though.
Comment 3 Alp Toker 2008-01-01 02:10:37 PST
Comment on attachment 18191 [details]
Apply GTK coding style

Mitch,

I've been thinking about this one the last couple of days.

There are two reasons why I don't think we should do this (yet):

(1) The patch makes indentation dependent on the length of various types names and variables names that we're planning to change pretty soon. So it would make future patches much larger than necessary. We want to keep patches as small as possible to ease reviewing and to ease merging, since several developers currently maintain their own branches and push regularly.

(2) The Apple guys often help out in maintaining this code, which is very valuable to the GTK+ port. I think they'd be less likely to casually fix our build if they had to mess about with whitespace in surrounding unrelated lines.

The GTK+ coding style is more aesthetic but I don't think it's worthwhile given these factors.

It may make sense to apply the header changes after the API has stabilised.

It probably makes sense to apply the part of the patch to GtkLauncher's main.c right now. I can apply a reduced version of the patch to do that if you like.
Comment 4 Michael Natterer 2008-01-02 07:07:56 PST
Sure, please go ahead and apply the patch to the launcher please, better
that than nothing :)

However, I don't really agree on the "we want to keep patches as small
as possible" argument. If you apply that policy, the coding stlye will
always be slightly messed up. Also, there is never a moment where you
can "safely" change a project's coding style. People always have
patches and branches.

I vote for better now than later as the API is guaranteed to grow,
and if it's in the right style now people are at least shown what
the right style is, rather than having their commits changed at
some later point and sitting on even larger patches to a bigger
code base which don't apply any longer.
Comment 5 Alp Toker 2008-01-08 12:06:03 PST
Thanks for your comments Mitch -- I think we should keep this debate open. There are strong arguments in each direction from both camps, one experienced in WebKit/C++ and the other in GTK+/C.

Our project sits at the borderline and the current coding style is a compromise that's worked quite well. As the project grows I'd be happy to re-evaluate this decision.
Comment 6 Christian Dywan 2008-06-07 13:43:42 PDT
Created attachment 21564 [details]
Apply gtk+ style to headers

This patch applies Gtk+ coding style to public webkit headers. It doesn't touch any implementation or private files.
Comment 7 Darin Adler 2008-06-08 13:57:47 PDT
Comment on attachment 21564 [details]
Apply gtk+ style to headers

rs=me
Comment 8 Christian Dywan 2008-06-10 00:51:38 PDT
Comment on attachment 21564 [details]
Apply gtk+ style to headers

Committed in r34477.