[webkit-reviews] review granted: [Bug 52253] remember diffstate for review tool : [Attachment 78598] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 52253: remember diffstate for review tool
https://bugs.webkit.org/show_bug.cgi?id=52253

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78598&action=review

This patch is fine.  It could use slightly more polish though.

> Websites/bugs.webkit.org/code-review.js:744
> +    updateDiffState();

updateDiffState sounds like I can call this whenever I like, but this is really
a load-time function.  Maybe loadDiffState?

> Websites/bugs.webkit.org/code-review.js:749
> +    if (localStorage.diffstate != 'sidebyside')
> +	 return;

It seems like we should be more explicit here.	Maybe check that we've gotten
one of the two expected values and then take action.

> Websites/bugs.webkit.org/code-review.js:753
> +    $('.FileDiff').each(function() {
> +	 convertFileDiff('sidebyside', this);
> +    });

This chunk of code seems like it should be shared with the side-by-side link,
right?