[webkit-reviews] review denied: [Bug 33153] Running multiple instances of run-webkit-tests is not possible on the same machine : [Attachment 45802] 1st step: change hardcoded /tmp to File::Spec->tmpdir()
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 4 09:16:30 PST 2010
Darin Adler <darin at apple.com> has denied Andras Becsi
<abecsi at inf.u-szeged.hu>'s request for review:
Bug 33153: Running multiple instances of run-webkit-tests is not possible on
the same machine
https://bugs.webkit.org/show_bug.cgi?id=33153
Attachment 45802: 1st step: change hardcoded /tmp to File::Spec->tmpdir()
https://bugs.webkit.org/attachment.cgi?id=45802&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> +my $httpdPidDir = File::Spec->tmpdir()."/WebKit";
We normally put spaces around operators like the "." here.
> - mkdir "/tmp/WebKit";
> + mkdir "$httpdPidDir";
It doesn't make sense to keep the quote marks here.
> + if (-f "$httpdPidFile") {
Or here.
> - kill 15, `cat /tmp/WebKit/httpd.pid` if -f "/tmp/WebKit/httpd.pid";
> + kill 15, `cat "$httpdPidFile"` if -f "$httpdPidFile";
Or here on the argument to -f.
> + rmtree "$httpdPidDir";
Or here.
I am surprised the old code did not remove the pid directory.
I think it would be more elegant to call unlink on the file and then rmdir on
the directory instead of using rmtree here.
I'm going to say review- even though my comments are only minor style things.
It would be nice to make it a little cleaner.
More information about the webkit-reviews
mailing list