[Webkit-unassigned] [Bug 117129] [GTK] Parameters 'inResult' and 'resolver' from function 'webkit_dom_document_evaluate' should be allowed to be NULL

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 4 08:23:37 PDT 2013


https://bugs.webkit.org/show_bug.cgi?id=117129


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #203548|review?                     |review-
               Flag|                            |




--- Comment #4 from Xan Lopez <xan.lopez at gmail.com>  2013-06-04 08:22:10 PST ---
(From update of attachment 203548)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list