[webkit-dev] Some new coding style rules

Darin Adler darin at apple.com
Wed Aug 4 10:43:20 PDT 2010


On Aug 4, 2010, at 1:29 AM, Nikolas Zimmermann wrote:

> 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

My personal preference: We should omit it.

> 2. ENABLE(FOO) #endif comments
> 
> #if ENABLE(FOO)
> ..
> #endif // ENABLE(FOO)
> 
> Shall we remove the comment, or require it explicitely in the style rules?

My personal preference: We should omit it.

> 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 think the former, as it makes no sense to process the header, if I already know it's enclosed in ENABLE(FOO) blocks)

My personal preference: The latter. One of the main reasons we include "Foo.h" first in "Foo.cpp" is to test that the header has the proper includes to be used standalone. It may be useful to do that test even if the header is supposed to be wrapped entirely in ENABLE(FOO).

But this is a close call. I’d be happy either way.

    -- Darin



More information about the webkit-dev mailing list