[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