[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

Attachment 8725: Patch v1

------- 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
- 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
- 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