[webkit-reviews] review denied: [Bug 62211] [CMAKE] Replace "; " with space in FEATURE_DEFINES macro : [Attachment 97060] Prototyped Patch

[bugzilla-daemon at webkit.org]

Patrick R. Gansterer <paroga at paroga.com> has denied Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 62211: [CMAKE] Replace ";" with space in FEATURE_DEFINES macro
https://bugs.webkit.org/show_bug.cgi?id=62211

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

------- Additional Comments from Patrick R. Gansterer <paroga at paroga.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=97060&action=review

> Source/WebCore/CMakeLists.txt:2213
> +SET(${FEATURE_DEFINES_WITH_SPACE_SEPARATOR} "")

Did you try this? ${FEATURE_DEFINES_WITH_SPACE_SEPARATOR} evalueats to space,
so you set the empty variable name. :-)
I'm not very happy with the variablename, but since I don't know a better one,
it's ok for me.

> Source/WebCore/CMakeLists.txt:2214
> +FOREACH(f ${FEATURE_DEFINES})

A common way for "temporary" variables is the start with underscore in CMake.
So "_feature" would be a better name instead of "f".

> Source/WebCore/ChangeLog:9
> +	   as separator. So, replace ";" separator with space in new macro.

Not sure if you "replace" in the new patch. Maybe you can update the ChangeLog?