[webkit-reviews] review granted: [Bug 37489] Use pretty patch for confirming webkit-patch diffs : [Attachment 53336] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 14 09:20:33 PDT 2010


Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 37489: Use pretty patch for confirming webkit-patch diffs
https://bugs.webkit.org/show_bug.cgi?id=37489

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think a more natural API would be to close the file in the patch-prep
function and return just a path.  The problem is we'd have to move away from a
temp function which deletes automatically on close (which I think woudl be a
good thing).

Is:
args = ["ruby","-I", pretty_patch_path, prettify_path]
not PEP8 compliant?  Personally I find such a short command easier to read on
one line.

Please consider making pretty_diff_file return a path instead of a file, I
think that woudl be much cleaner (and better supports future usecases of having
a "webkit-patch pretty-diff" command!  The current tmp-hack isn't very useful.


More information about the webkit-reviews mailing list