[webkit-reviews] review granted: [Bug 27323] Better support for non-Cygwin SVN on Windows : [Attachment 32963] Fixes, part 4

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 17 12:58:27 PDT 2009


David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Peter Kasting
<pkasting at google.com>'s request for review:
Bug 27323: Better support for non-Cygwin SVN on Windows
https://bugs.webkit.org/show_bug.cgi?id=27323

Attachment 32963: Fixes, part 4
https://bugs.webkit.org/attachment.cgi?id=32963&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> +my $devnull = File::Spec->devnull();

I think $devNull would probably read a bit better since it's technically two
words.	(I would use $dev_null in Python.)

> -	   $mimeType =~ s/\r?\n$//g;
> +	   $mimeType =~ s/\r?\n?$//g;

This pattern seems a bit strange since it could end up replacing nothing.  I
think this pattern might work better:

	$mimeType =~ s/[\r\n]+$//g;

> -	   $line =~ s/\r?\n$//g;
> +	   $line =~ s/\r?\n?$//;

Ditto.

> -	   print `svn cat ${sourceFile} | diff -u /dev/null - | tail -n +3`;
> +	   print `svn cat ${sourceFile} | diff -u $devnull - | tail -n +3`;

$devNull

> +	   # Ignore error messages in case we've run past the root of the
checkout.
> +	   open INFO, "svn info '$path' 2> $devnull |" or die;

$devNull

>	   while (<INFO>) {
>	       if (/^Repository Root: (.+)/) {
>		   $thisRoot = $1;
> +		   { local $/ = undef; <INFO>; }  # Eat the rest of the input.
>	       }
>	   }

I think "Consume" would be better than "Eat" here, unless you're feeling
hungry.  :)

r=me


More information about the webkit-reviews mailing list