[webkit-reviews] review denied: [Bug 23332] Fix some skia pattern creation issues : [Attachment 26732] PatternSkia fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 14 16:05:30 PST 2009


Eric Seidel <eric at webkit.org> has denied Michael Moss <mmoss at chromium.org>'s
request for review:
Bug 23332: Fix some skia pattern creation issues
https://bugs.webkit.org/show_bug.cgi?id=23332

Attachment 26732: PatternSkia fix
https://bugs.webkit.org/attachment.cgi?id=26732&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
WebKit style doesn't put {} around single-line if clauses.

However in this case, it seems an early-return would be more appropriate.  if
normal repeating case can early-return a shader, and then no else is needed
after the return for the more complicated case (thus not indenting an extra 4
spaces).

Also, WebKit uses FIXME instead of TODO with no username attribution.

WebKit also doesn't wrap to 80cols like Google does.

I'm surprised bm2.eraseARGB(0x00, 0x00, 0x00, 0x00); doesn't take a color
argument instead.

WebKit's style is a bit strange (probably less so than Google's though), but at
least it's documented:
http://webkit.org/coding/coding-style.html

r- for the style issues.  Otherwise looks fine.


More information about the webkit-reviews mailing list