WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2010-04-11 00:15:23 PDT
Created
attachment 53079
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug