[webkit-reviews] review denied: [Bug 63381] [GTK] [WK2] Implement missing initializeLogChannel function. : [Attachment 98589] Logging - WK2 GTK

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Lukasz Slachciak
<l.slachciak at samsung.com>'s request for review:
Bug 63381: [GTK] [WK2] Implement missing initializeLogChannel function.
https://bugs.webkit.org/show_bug.cgi?id=63381

Attachment 98589: Logging - WK2 GTK
https://bugs.webkit.org/attachment.cgi?id=98589&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98589&action=review

Nice. Some comments below.

> Source/WebKit2/Platform/Logging.cpp:48
> +WTFLogChannel* getChannelFromName(const String& channelName)

Why not just put this in LoggingGtk and make it static?

> Source/WebKit2/Platform/gtk/LoggingGtk.cpp:44
> +    // unfortunatelly this is initialized and enumerated for every
WTFLogChannel

This seems like an expensive thing to do for every log channel. Why not just do
it statically?

> Source/WebKit2/Platform/gtk/LoggingGtk.cpp:45
> +    gchar** logv = g_strsplit(logEnv, " ", -1);

I would prefer you to do this with WTF::String instead of GLib. You can avoid
manual memory management that way.

> Source/WebKit2/Platform/gtk/LoggingGtk.cpp:56
> +#endif /* !LOG_DISABLED */

C++ style comment here, I think.