[webkit-reviews] review granted: [Bug 130174] [GTK] Fix unused parameter warnings in the GObject WebKitDOM bindings : [Attachment 226572] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 130174: [GTK] Fix unused parameter warnings in the GObject WebKitDOM
bindings
https://bugs.webkit.org/show_bug.cgi?id=130174

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=226572&action=review


OK.

> Source/WebCore/bindings/gobject/WebKitDOMObject.cpp:28
> +    switch (propertyId) {
>      default:
> -	   G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> +	   G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propertyId, pspec);
>	   break;
>      }

The switch statement is unnecessary as we always execute the code for the
default case.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:661
> +	   push(@txtGetProps, "#else\n") if $conditionalString;
> +	   push(@txtGetProps, "    UNUSED_PARAM(value);\n") if
$conditionalString;
> +	   push(@txtGetProps, "${conditionGuardEnd}\n") if $conditionalString;

Unless you feel the duplication of the if conditional ("if $conditionalString")
improves the readability of this code, I would write this as:

if ($conditionalString) {
    push(@txtGetProps, "#else\n");
    push(@txtGetProps, "    UNUSED_PARAM(value);\n");
    push(@txtGetProps, "${conditionGuardEnd}\n");
}

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:674
> +	       push(@txtSetProps, "#else\n") if $conditionalString;
> +	       push(@txtSetProps, "    UNUSED_PARAM(value);\n") if
$conditionalString;
> +	       push(@txtSetProps, "${conditionGuardEnd}\n") if
$conditionalString;

Similarly, I would write this as:

if ($conditionalString) {
    push(@txtSetProps, "#else\n");
    push(@txtSetProps, "    UNUSED_PARAM(value);\n");
    push(@txtSetProps, "${conditionGuardEnd}\n");
}

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:797
> +	   push(@cBodyProperties, "UNUSED_PARAM(request);\n");

Nit: We should indent "UNUSED_PARAM(request);".

> Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNode.cpp:101
> +UNUSED_PARAM(request);

Nit: This line should be indented 4 spaces.