[webkit-reviews] review denied: [Bug 54627] [GStreamer] URI queries support in webkitwebsrc : [Attachment 82777] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Andoni <ylatuya at gmail.com>'s
request for review:
Bug 54627: [GStreamer] URI queries support in webkitwebsrc
https://bugs.webkit.org/show_bug.cgi?id=54627

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

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

Thanks for your contribution! Please consider the following review comments.
This patch is only a few changes from an r+.

> WebCore/ChangeLog:10
> +	   [GStreamer] URI queries support in webkitwebsrc
> +	   https://bugs.webkit.org/show_bug.cgi?id=54627
> +
> +	   * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
> +	   (webkit_web_src_init):
> +	   (webKitWebSrcQuery):

Do you mind explaining a bit what this adds? Those familiar with Gstreamer will
understand what sry query support means, but not the majority of the people
skimming the ChangeLog.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:120
> +static gboolean webKitWebSrcQuery(GstPad * pad, GstQuery * query);

No spaces before the asterisks here.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:493
> +
> +static gboolean webKitWebSrcQuery(GstPad * pad, GstQuery * query)
> +{
> +    WebKitWebSrc * src = WEBKIT_WEB_SRC(gst_pad_get_parent_element(pad));

Ditto.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:494
> +    gboolean ret;

Normally we don't abbreviate variable names in WebKit, so perhaps something
like "success"?