[webkit-reviews] review requested: [Bug 9322] Teach
svn-create-patch to sort its output : [Attachment 8725] Patch v1
bugzilla-request-daemon at opendarwin.org
bugzilla-request-daemon at opendarwin.org
Mon Jun 5 22:36:14 PDT 2006
David Kilzer (ddkilzer) <ddkilzer at kilzer.net> has asked for review:
Bug 9322: Teach svn-create-patch to sort its output
http://bugzilla.opendarwin.org/show_bug.cgi?id=9322
Attachment 8725: Patch v1
http://bugzilla.opendarwin.org/attachment.cgi?id=8725&action=edit
------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at kilzer.net>
I took no prisoners in cleaning up the perl code, but I may have been too
aggressive. (I was tempted to remove 'use' statements, too, but held off.)
Changes (in no particular order):
- Sorted 'use' statements alphabetically.
- Reworded some die() messages.
- Extracted path canonicalization to canonicalizePath() method and switched to
using standard Perl modules instead of regular expressions assuming forward
slashes.
- Moved non-subroutine code together (or moved all subroutines to the end of
the file) and added an 'exit 0;' statement so you know where the main code
ends. Subroutines are alphabetized.
- Moved list-of-path cleanup into the else block of the first if/else statement
because it's only needed there. If no arguments are given, "." is used by
default and is the only path needed. Changed a while() loop to a for() loop
since I thought it read better; also replaced another regex with dirname().
- Created new generateFileList() subroutine. It uses "svn diff" instead of
"svn stat" since (a) "svn diff" was roughly 4x faster than "svn stat" in a
local test and (b) it's easier to know if a file is binary or not using "svn
diff".
- Completely gutted diff() and renamed to generateDiff(). It only has to
handle creating a patch for one file at a time now. I also removed all of the
chdir() business since Subversion handles the full paths quite nicely. That
cleaned up a lot of logic in the patch path fix-up as well. Should I have kept
this code?
More information about the webkit-reviews
mailing list