[webkit-reviews] review denied: [Bug 117129] [GTK] Parameters 'inResult' and 'resolver' from function 'webkit_dom_document_evaluate' should be allowed to be NULL : [Attachment 203548] Patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Diego Pino <dpino at igalia.com>'s
request for review:
Bug 117129: [GTK] Parameters 'inResult' and 'resolver' from function
'webkit_dom_document_evaluate' should be allowed to be NULL
https://bugs.webkit.org/show_bug.cgi?id=117129

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

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=203548&action=review


Looks pretty good in general, just a few small comments.

>> Source/WebCore/ChangeLog:5
>> +	    * Add test for function 'webkit_dom_document_evaluate'
> 
> Need whitespace between colon and description 
[changelog/filechangedescriptionwhitespace] [5]

I think the style bot is complaining because no text should appear above the
title/uri combo.

> Source/WebKit/gtk/ChangeLog:5
> +	   Add test for function 'webkit_dom_document_evaluate'

Although no warning here, so not sure. Still this seems wrong.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:49
> +my $can_be_null_params = {

I think methods have to be in camel case.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:864
> +	   }

I think you can leave the original line and just prepend the !$paramName ||
thing if ParamCanBeNull is true, right?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:885
> +    }

Can this actually happen?

> Source/WebKit/gtk/tests/testdomdocument.c:227
> +	   0, NULL, NULL);

I'd just leave this in the previous line.