[webkit-reviews] review denied: [Bug 57992] old-run-webkit-tests: add support for audio files : [Attachment 95524] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 1 10:10:22 PDT 2011


Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 57992: old-run-webkit-tests: add support for audio files
https://bugs.webkit.org/show_bug.cgi?id=57992

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95524&action=review

> Tools/Scripts/old-run-webkit-tests:2305
> +	       } elsif ($lineIn =~ /(.*)#EOF$/) {
> +		   if ($1) {

This will do the wrong thing if $1 is "0".  Perl silently converts that to a
number, which then becomes false.  You could just compare to "".

> Tools/Scripts/old-run-webkit-tests:2336
> +    my $joined_error = join("", @error);

Nit: Pulling this out into a variable doesn't seem to add any value.


More information about the webkit-reviews mailing list