[webkit-reviews] review denied: [Bug 103605] [EFL] uniformed coding style in cmake files : [Attachment 176659] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 29 19:27:15 PST 2012


Laszlo Gombos <laszlo.gombos at webkit.org> has denied Halton Huo
<halton.huo at gmail.com>'s request for review:
Bug 103605: [EFL] uniformed coding style in cmake files
https://bugs.webkit.org/show_bug.cgi?id=103605

Attachment 176659: Patch
https://bugs.webkit.org/attachment.cgi?id=176659&action=review

------- Additional Comments from Laszlo Gombos <laszlo.gombos at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176659&action=review


Thank you for the initiative. I do not think anyone would object trying to
unify the style, but there needs to be an agreement on what the style is. I CCd
a few more folks who might have input to the style.

r- for now as at least the ChangeLog needs to be fixed, but more importantly
I'd like to hear from other stakeholders before this lands.

> ChangeLog:3
> +	   [EFL] uniformed coding style in cmake files

If the intent is to change all files that are input to CMake you should change
the prefix of the bug to [CMake].

We usually start the title with a capital letter; 

"Unify coding style for CMake files" sounds better to me for a title.

> ChangeLog:8
> +	   Uniform coding style for .cmake files with rules:

What about other files that CMake takes as input - e.g. CMakeLists.txt ?

> ChangeLog:12
> +	   - 4-space as indent, no TAB
> +	   - Uppercase and no space when call macros: MESSAGE("testing")
> +	   - 1-space after condition clause: IF (), ELSE (), ENDIf(), FOREACH
()
> +	   - 1-space after macro definition: MACRO (), ENDMACRO ()

Where are all these rules coming from ? Are these custom for other projects as
well ? Have you looked at other styles, like
http://techbase.kde.org/Policies/CMake_Coding_Style (i just picked one, it does
not mean that that is the one we should take as a base).

> ChangeLog:19
> +	   * Source/cmake/WebKitPackaging.cmake:

Again, what about CMakeLists.txt ?


More information about the webkit-reviews mailing list