[webkit-reviews] review denied: [Bug 35057] Set viewport to canvas size upon context creation : [Attachment 49459] revised patch: changed the log text according to Ken's advice

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 35057: Set viewport to canvas size upon context creation
https://bugs.webkit.org/show_bug.cgi?id=35057

Attachment 49459: revised patch: changed the log text according to Ken's advice
https://bugs.webkit.org/attachment.cgi?id=49459&action=review

------- Additional Comments from David Levin <levin at chromium.org>
> Index: WebCore/ChangeLog
> +2010-02-24  Zhenyao Mo  <zmo at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Set viewport to canvas size upon context creation
> +	   https://bugs.webkit.org/show_bug.cgi?id=35057
> +
> +	   Covered by existing tests, in particular
fast/canvas/webgl/gl-get-calls.html
> +
> +	   * html/canvas/WebGLRenderingContext.cpp:
> +	   (WebCore::WebGLRenderingContext::WebGLRenderingContext):

Ideally there are per file comments explaining the change. For example,

    (WebCore::WebGLRenderingContext::WebGLRenderingContext): Set the viewport
siz


> Index: LayoutTests/ChangeLog
> +2010-02-24  Zhenyao Mo  <zmo at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Need a short description and bug URL (OOPS!)

Do you see what I see? :)



> +
> +	   * fast/canvas/webgl/bug-32888.html:

Per file comments would be nice. For example "Removed call to set the viewport
size since this is done when the context is created."

(You may say "Ditto." if the comment for a file is the same as the last
comment.

> +	   * fast/canvas/webgl/gl-get-calls-expected.txt:

A comment for this file would have a *very short* explanation of why the result
changed.

> +	   * fast/canvas/webgl/gl-get-calls.html:
> +	   * fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html:
> +	   * fast/canvas/webgl/tex-sub-image-2d.html:
> +	   * fast/canvas/webgl/texImage2DImageDataTest.html:
> +	   * fast/canvas/webgl/triangle.html:
> +	   * fast/canvas/webgl/viewport-unchanged-upon-resize.html:
> +