[webkit-reviews] review granted: [Bug 172688] [WebIDL] Generate named property deleters : [Attachment 311485] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 172688: [WebIDL] Generate named property deleters
https://bugs.webkit.org/show_bug.cgi?id=172688

Attachment 311485: Patch

https://bugs.webkit.org/attachment.cgi?id=311485&action=review




--- Comment #15 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 311485
  --> https://bugs.webkit.org/attachment.cgi?id=311485
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311485&action=review

> Source/WebCore/bindings/js/JSDOMAbstractOperations.h:38
> +template<bool overrideBuiltins, class JSClass>

Could we use an enum class instead of a boolean? I understand this is only used
in generated code but the call sites are really not very readable.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:822
> +	   push(@$outputArray, "	return
!impl.isSupportedPropertyIndex(index.value()) ? true : false;\n");

return !impl.isSupportedPropertyIndex(index.value()); ? Why the ternary?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:829
> +    # the remained of the algoritm ourselves.

typo: algorithm

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:856
> +	   push(@$outputArray, "    return
!impl.isSupportedPropertyIndex(index) ? true : false;\n");

Why the ternary?

> Source/WebCore/storage/Storage.h:50
> +    // Bindings support functions

Missing period at the end