[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