RESOLVED FIXED 37406
Add PrettyPatch links to new-run-webkit-tests output
https://bugs.webkit.org/show_bug.cgi?id=37406
Summary Add PrettyPatch links to new-run-webkit-tests output
Eric Seidel (no email)
Reported 2010-04-11 00:10:29 PDT
Add PrettyPatch links to new-run-webkit-tests output
Attachments
Patch (14.02 KB, patch)
2010-04-11 00:15 PDT, Eric Seidel (no email)
no flags
Eric Seidel (no email)
Comment 1 2010-04-11 00:15:23 PDT
Adam Barth
Comment 2 2010-04-11 00:23:54 PDT
Comment on attachment 53079 [details] Patch Ok. I miss the pretty diffs. Glad it's coming back.
WebKit Commit Bot
Comment 3 2010-04-11 00:42:03 PDT
Comment on attachment 53079 [details] Patch Clearing flags on attachment: 53079 Committed r57446: <http://trac.webkit.org/changeset/57446>
WebKit Commit Bot
Comment 4 2010-04-11 00:42:09 PDT
All reviewed patches have been landed. Closing bug.
Dirk Pranke
Comment 5 2010-04-11 11:35:35 PDT
Comment on attachment 53079 [details] Patch > diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog > index b1e0733..33893f6 100644 > --- a/WebKitTools/ChangeLog > +++ b/WebKitTools/ChangeLog > @@ -1,3 +1,27 @@ > +2010-04-11 Eric Seidel <eric@webkit.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Add PrettyPatch links to new-run-webkit-tests output > + https://bugs.webkit.org/show_bug.cgi?id=37406 > + Does this patch introduce a dependency on Ruby into the test system? I don't think we have Ruby on Chromium windows (and am not sure about Linux, although it's probably there). Also, do we really need three different kinds of diffs? > @@ -519,6 +519,7 @@ class Port(object): > '--end-insert=##WDIFF_END##', > actual_filename, > expected_filename] > + # FIXME: Why not just check os.exists(executable) once? > global _wdiff_available See the comments in base.py's Port.wdiff_text()
Eric Seidel (no email)
Comment 6 2010-04-11 11:37:46 PDT
I need to add a check for Ruby, as it looks like the Linux canary doesn't have ruby. Personally I find PrettyPatch diffs prettier than wdiff. :) wdiff doesn't exist on Mac systems, and more importantly is the same prettydiff that we use on bugs.webkti.org (the "Formatted Diff" link next to any patch is a PrettyPatch diff).
Dirk Pranke
Comment 7 2010-04-11 11:43:14 PDT
(In reply to comment #6) > I need to add a check for Ruby, as it looks like the Linux canary doesn't have > ruby. > > Personally I find PrettyPatch diffs prettier than wdiff. :) wdiff doesn't > exist on Mac systems, and more importantly is the same prettydiff that we use > on bugs.webkti.org (the "Formatted Diff" link next to any patch is a > PrettyPatch diff). Sorry, I should have been clearer; I'm not wedded to wdiff either, except that it shows you intraline diffs, and I don't know that anyone else is, either. Perhaps we should replace wdiff with PrettyPatch, instead of having three diffs?
Adam Barth
Comment 8 2010-04-11 11:53:59 PDT
> Perhaps we should replace wdiff with PrettyPatch, instead of having three > diffs? That sounds like a good plan to me. That's consistent with how the tool has worked historically and with how bugs.webkit.org works.
Eric Seidel (no email)
Comment 9 2010-04-11 18:47:48 PDT
I wont be able to fix the script for pllatforms missing Ruby until tomorrow morning. If that's blocking the Gardner from getting work done, feel free to roll this out.
Eric Seidel (no email)
Comment 10 2010-04-12 01:10:09 PDT
Fixed systems missing ruby in bug 37441.
Note You need to log in before you can comment on or make changes to this bug.