[webkit-reviews] review denied: [Bug 29802] [Qt] Make build-webkit script work on Windows : [Attachment 40225] proposed patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Csaba Osztrogonác
<oszi at inf.u-szeged.hu>'s request for review:
Bug 29802: [Qt] Make build-webkit script work on Windows
https://bugs.webkit.org/show_bug.cgi?id=29802

Attachment 40225: proposed patch
https://bugs.webkit.org/attachment.cgi?id=40225&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
Overall, this is a nice clean-up!  Comments below.

> +    my $mkdirParentsSwitch = (($^O eq "MSWin32") ? "" : "-p");

A couple things here:

- Instead of making a custom $mkdirParentsSwitch variable, I'd rather see you
add an @mkdirArgs variable, and only add the "-p" switch to it if necessary.

my @mkdirArgs;
push @mkdirArgs, "-p" if !isWindows();

- The ($^O eq "MSWin32") test should really be isWindows() instead.  (If you
don't want to test for Cygwin, then fix isWindows() to only test for Windows,
and update run-webkit-test to use "isCygwin() || isWindows()" in the place
where it uses isWindows() now.)

The rest of the changes look good!

r- to address the above issues.