[webkit-reviews] review denied: [Bug 124661] [GTK] Release compilation fails when defining "LOG_DISABLED=0" : [Attachment 217438] Patch

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has denied Andres Gomez Garcia
<agomez at igalia.com>'s request for review:
Bug 124661: [GTK] Release compilation fails when defining "LOG_DISABLED=0"
https://bugs.webkit.org/show_bug.cgi?id=124661

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

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=217438&action=review


> Source/WebCore/html/HTMLTrackElement.cpp:36
> +#if !LOG_DISABLED

The usage of the CString class inside this file is not behind any !LOG_DISABLED
guard so I think you should not put the include behind it either, because if
you do it so it would be because you know how that macro is being defined, and
not based in the information you have by looking at this implementation file
only.

So, even though calls to LOG() won't ever be translated to anything requiring
CString if !LOG_DISABLED (and that's why it does not fail otherwise), I believe
it's better to be consistent and just include CString normally, as it's done in
other places (e.g. HTMLMediaElement.cpp)