[webkit-reviews] review denied: [Bug 49692] Rebaseline server: add loupe for image diffs : [Attachment 74163] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 17 16:26:09 PST 2010


Tony Chang <tony at chromium.org> has denied Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 49692: Rebaseline server: add loupe for image diffs
https://bugs.webkit.org/show_bug.cgi?id=49692

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74163&action=review

This sounds awesome.  I'll have to try the rebaseline tool after this lands. 
r- for naming style, otherwise, this looks good.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js:37
> +    this.node_ = $('loupe');
> +    this.currentCornerX_ = -1;
> +    this.currentCornerY_ = -1;

There's no real webkit style for JS objects.  Ojan and I discussed briefly and
he suggested just using _node and _currentCornerX for the following reasons:
1) JS outside of google seems to use prefix _ more than suffix _.
2) It's similar to PEP8 and JS seems more like python than C++.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js:55
> +Loupe.prototype.handleOutputClick_ = function(event)

To go with the above recommendation, this would be _handleOutputClick.


More information about the webkit-reviews mailing list