[webkit-reviews] review denied: [Bug 123439] Fix reported build warnings for GTK : [Attachment 215916] patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Tibor Mészáros
<mtibor at inf.u-szeged.hu>'s request for review:
Bug 123439: Fix reported build warnings for GTK
https://bugs.webkit.org/show_bug.cgi?id=123439

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

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215916&action=review


Thanks for the patch. Please make sure the patch applies to current trunk.
Since the warnings fixed a quite different, I think it would be better to split
the patch and open a bug report for every patch. One for compile warnings,
since the leveldb is not specific to GTK, another one for documentation
warnings and another one for deprecated api used (if appropriate, if the only
deprecated api used is gdk_threads_enter/leave see bug #120070).

> Source/ThirdParty/leveldb/table/table.cc:-231
> -	 Slice handle = iiter->value();

leveldb code is not specific to GTK+. Since this is in third_party maybe we
should update leveldb to a newer version if warnings are fixed or fix the
warnings upstream.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:-77
> -void webkit_dom_html_element_set_id(WebKitDOMHTMLElement* element, const
gchar* value)
> -{
> -    g_warning("The set_id method on WebKitDOMHTMLElement is deprecated. Use
the one in WebKitDOMElement instead.");
> -    webkit_dom_element_set_id(WEBKIT_DOM_ELEMENT(element), value);
> -}

What compile warnings are causing these methods?

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:32
> - * Returns:
> + * Returns: (transfer none):

This is a workaround, gtk-doc doesn't complain because there's something after
the Returns:, but the annotation doesn't make sense for a method returning a
gboolean. This could be fixed adding something like 

Returns: a #gboolean

I think this was already fixed in r158200 in any case.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:40
> - * Returns:
> + * Returns: (transfer none):

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:59
> - * Returns:
> + * Returns: (transfer none):

Ditto.

> Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:-115
> -WEBKIT_API gboolean
webkit_dom_event_target_add_event_listener_with_closure(WebKitDOMEventTarget
*target,
> -									      
const char	     *event_name,
> -									      
GClosure	     *handler,
> -									      
gboolean	      use_capture);

This is valid API, why are you removing it?

> Source/WebCore/bindings/gobject/WebKitDOMEventTarget.h:-134
> -WEBKIT_API gboolean
webkit_dom_event_target_remove_event_listener_with_closure(WebKitDOMEventTarget
*target,
> -									       
  const char	       *event_name,
> -									       
  GClosure	       *handler,
> -									       
  gboolean		use_capture);

Ditto.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:960
> -    if (IsGDOMClassType($function->signature->type)) {
> +    if (IsGDOMClassType($function->signature->type) && $returnType ne
"void") {

I'm pretty sure this has already been fixed too in r158200