[webkit-dev] Some new coding style rules

[Jeremy Orlow]

On Wed, Aug 4, 2010 at 9:29 AM, Nikolas Zimmermann <
zimmermann at physik.rwth-aachen.de> wrote:

> Good morning WebKit crowd,
>
> I'd like to discuss some style issues, that I'm frequently unsure about:
>
> 1. namespace closing brace
> It was discussed in
> http://article.gmane.org/gmane.os.opendarwin.webkit.devel/10563, but with
> no real result.
>
> When writing headers, do we need the "// namespace Foo" comment after the
> namespace closing brace? I think it's visual noise and would like to see a
> style rule that forbids it.
> (A rule that says we need this would also be fine with me, as long as we
> write it down. But I'd vote for removing those comments, I don't trust them
> anyways, if I'm unsure.)
>
> namespace WebCore {
> ...
> } // namespace WebCore
>
> 2. ENABLE(FOO) #endif comments
>
> #if ENABLE(FOO)
> ..
> #endif // ENABLE(FOO)
>
> Shall we remove the comment, or require it explicitely in the style rules?
>
> 3. Inclusion order
>
> Say Foo.h is guarded with ENABLE(FOO) macros
> Foo.h:
> #if ENABLE(FOO)
> namespace WebCore {
>
> class Foo...
>
> }
> #endif
>
> What's the correct inclusion order in Foo.cpp:
> #include "config.h"
> #include "Foo.h"
>
> #if ENABLE(FOO)
> ..
> #endif
>
> or
>
> #include "config.h"
> #include "Foo.h"
>
> #if ENABLE(FOO)
> ..
> #endif
>

I don't see the difference between these two examples.

I'm not sure if this is what you were asking, but here's another: Should we
do this:

#include "config.h"
#include "Foo.h"

#if ENABLE(FOO)

#include "other"
#include "includes"

...

#endif

or this

#include "config.h"
#include "Foo.h"

#include "other"
#include "includes"

#if ENABLE(FOO)

...

#endif

or this

#include "config.h"

#if ENABLE(FOO)
#include "Foo.h"

#include "other"
#include "includes"

...

#endif

The last one is definitely required in custom JS* or V8* files since the
Foo.h file will only exist if the guard is enabled.  But besides this one
case where we have to, I feel splitting the #include config from the main
header file adds unnecessary noise and is ugly.  So, for the common case,
I'd like to suggest we go with my first example since it doesn't include a
bunch of unnecessary files, but it keeps our standard convention even in
(the many) files that are entirely encapsulated in an ENABLE block.

I'd agree that it'd be nice to get some of this stuff nailed down so
we're consistent.


> (I think the former, as it makes no sense to process the header, if I
> already know it's enclosed in ENABLE(FOO) blocks)
>
> I'd be happy if we could agree on rules and write them down.
>
> Cheers,
> Niko
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100804/b8033598/attachment.html>