[webkit-reviews] review denied: [Bug 111641] Conformance Test 1.0.3 (Beta) function: bufferData undefined value failed. : [Attachment 191905] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has denied Jason Anderssen
<janderssen at gmail.com>'s request for review:
Bug 111641: Conformance Test 1.0.3 (Beta) function: bufferData undefined value
failed.
https://bugs.webkit.org/show_bug.cgi?id=111641

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

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=191905&action=review


These may seem like nitpicking comments (and they are) but WebKit is especially
strict about coding style. Sorry. You get used to it :) Make these easy fixes
and the r+ is yours.

> Source/WebCore/ChangeLog:7
> +	   Reviewed by NOBODY (OOPS!).
> +

In this space you should try to describe the bug and change in a little more
detail. e.g. "The WebGL specification requires that buffer data cannot be of
zero length. This is a new test in the Khronos 1.0.3 WebGL test suite."

Also, you should reference the Khronos test here. Typically we require a test
for every WebKit commit, but in this case it's ok because we are in the process
of incorporating the Khronos tests into WebKit, so it will be there eventually.


> Source/WebCore/ChangeLog:9
> +	   (WebCore):

Remove this line.

> Source/WebCore/ChangeLog:10
> +	   (WebCore::WebGLRenderingContext::bufferData):

You should always try to explain what you did in the changelog. In this case it
is really easy - just something like "Synthesize error and return if size was
0."

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1115
> +	   // undefined in WEBGL will parse in 0.

No need for this comment. Also, for future reference, WebKit coding style
requires comments to start with a capital letter.