Bug 52180 - convert back to unified from sidebyside diff
Summary: convert back to unified from sidebyside diff
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-10 16:55 PST by Ojan Vafai
Modified: 2011-01-11 12:07 PST (History)
4 users (show)

See Also:

Attachments
Patch (11.57 KB, patch)
2011-01-10 16:58 PST, Ojan Vafai 		abarth: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2011-01-10 16:55:27 PST 
convert back to unified from sidebyside diff
Comment 1 Ojan Vafai 2011-01-10 16:58:00 PST 
Created attachment 78473 [details]
Patch
Comment 2 Adam Barth 2011-01-10 17:31:08 PST 
Comment on attachment 78473 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review

> Websites/bugs.webkit.org/code-review.js:885
> +  $('.unify-link').live('click', handleUnifyLinkClick);
>    $('.ExpandLink').live('click', handleExpandLinkClick);

These classes seem unnecessarily different.
Comment 3 Adam Barth 2011-01-10 17:31:29 PST 
It's difficult to review this patch for correctness, but it seems fine.
Comment 4 Ojan Vafai 2011-01-10 18:48:12 PST 
(In reply to comment #2)
> (From update of attachment 78473 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review
> 
> > Websites/bugs.webkit.org/code-review.js:885
> > +  $('.unify-link').live('click', handleUnifyLinkClick);
> >    $('.ExpandLink').live('click', handleExpandLinkClick);
> 
> These classes seem unnecessarily different.

Yeah. I never know which style to follow in this file. The existing style in this file is some mashup of webkit/chromium/bugzilla style. At some point we should make it have a consistent style. Until then, I'm not sure what style to follow, so I end up doing some mix. :(
Comment 5 Adam Barth 2011-01-10 18:56:28 PST 
Comment on attachment 78473 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review

>>> Websites/bugs.webkit.org/code-review.js:885
>>>    $('.side-by-side-link').live('click', handleSideBySideLinkClick);
>>> +  $('.unify-link').live('click', handleUnifyLinkClick);
>>>    $('.ExpandLink').live('click', handleExpandLinkClick);
>> 
>> These classes seem unnecessarily different.
> 
> Yeah. I never know which style to follow in this file. The existing style in this file is some mashup of webkit/chromium/bugzilla style. At some point we should make it have a consistent style. Until then, I'm not sure what style to follow, so I end up doing some mix. :(

Fair enough.  :)
Comment 6 Ojan Vafai 2011-01-11 10:39:32 PST 
Committed r75514: <http://trac.webkit.org/changeset/75514>
Comment 7 WebKit Review Bot 2011-01-11 12:07:12 PST 
http://trac.webkit.org/changeset/75514 might have broken GTK Linux 64-bit Debug