[webkit-reviews] review granted: [Bug 27204] svn-apply doesn't understand the executable bit : [Attachment 53067] Patch with unit tests

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 27204: svn-apply doesn't understand the executable bit
https://bugs.webkit.org/show_bug.cgi?id=27204

Attachment 53067: Patch with unit tests
https://bugs.webkit.org/attachment.cgi?id=53067&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Tab city batman!

You're right, parseStartOfPatchOrPropertyChange is not really a win.

+    if (my $filePath = parseStartOfPatchOrPropertyChange($_)) {
+	 if (/^Index: (.+)/) {
+	     $indexPath = $filePath;
+	 } else {
+	     $propertyChangePath = $filePath;
+	 }

would hav ebeen cleaner as just:

if (index thingy) {

} else (property thingy) {

}

instead of the double-check inside the function.  Not sure.  I mean, you could
have done two functions instead.

if (my filePath = parseStartOfPatch()) {

} else (my propertyName = prasePropertyChange()) {

}

Either way, I don't need to see this again.  But consider the above.