[webkit-reviews] review denied: [Bug 66646] ArrayBuffer should have slice method. : [Attachment 104951] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 18 12:02:42 PDT 2011
Kenneth Russell <kbr at google.com> has denied Shinya Kawanaka
<shinyak at google.com>'s request for review:
Bug 66646: ArrayBuffer should have slice method.
https://bugs.webkit.org/show_bug.cgi?id=66646
Attachment 104951: Patch
https://bugs.webkit.org/attachment.cgi?id=104951&action=review
------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=104951&action=review
Thanks for picking up this work. The code looks fine and the test looks great.
A couple of minor nits to clean up before committing. Also, please mark cq? on
the bug if you would like it committed after review.
> LayoutTests/ChangeLog:3
> + Added unittest for ArrayBuffer#slice
Please use the bug description verbatim along with the URL. If necessary,
provide detail in additional paragraphs.
> Source/WebCore/ChangeLog:3
> + Implemented ArrayBuffer#slice.
Same comment here.
> Source/WebCore/html/canvas/ArrayBuffer.cpp:33
> +static long clampValue(long x, long left, long right)
This should take and return ints instead of longs.
> Source/WebCore/html/canvas/ArrayBuffer.h:54
> + unsigned index(int idx) const;
This method name is a little confusing; what about "clampIndex"? Also, avoid
abbreviations (see http://www.webkit.org/coding/coding-style.html ), so the
argument should be named "index".
More information about the webkit-reviews
mailing list