[webkit-reviews] review granted: [Bug 116114] [Windows] Identify proper run environment for scripts : [Attachment 201754] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 116114: [Windows] Identify proper run environment for scripts
https://bugs.webkit.org/show_bug.cgi?id=116114

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

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=201754&action=review


r=me with added return statements.  Thanks!

> Tools/Scripts/webkitdirs.pm:421
> +    $programFilesPath = $ENV{'PROGRAMFILES(X86)'} || $ENV{'PROGRAMFILES'} ||
"C:\\Program Files";

We should probably have a 'return $programFilesPath;' after this line.

I know Perl will do the right thing here, but it's a little too non-obvious for
people who aren't Perl experts.

> Tools/Scripts/webkitdirs.pm:434
> +    chomp($vsInstallDir = `cygpath "$vsInstallDir"`) if isCygwin();

Ditto for return statement here.  (Actually, will chomp() do the right thing
here?)

> Tools/Scripts/webkitdirs.pm:443
> +    $vsVersion = ($installDir =~ /Microsoft Visual Studio ([0-9]+\.[0-9]*)/)
? $1 : "8";

Ditto for return statement here.