[webkit-reviews] review granted: [Bug 51642] Add WinCE support to build-webkit : [Attachment 77698] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 51642: Add WinCE support to build-webkit
https://bugs.webkit.org/show_bug.cgi?id=51642

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

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

r=me, but you need to fix up the space before the "-j", and please consider
renaming the $make variable.

> Tools/Scripts/webkitdirs.pm:1465
> +	   $makeArgs .= "-j" . numberOfCPUs() if ($makeArgs !~ m/-j\s*\d+/);

Still need a space before the "-j" here:  " -j".

> Tools/Scripts/webkitdirs.pm:1475
> -	   my $make = "make";
> +	   my $make = $cmakebin . " --build .";

Nit:  Using $make here is now confusing.  I would rename $make to something
like $cmakeBuildCommand.

> Tools/Scripts/webkitdirs.pm:1511
> +	   $result = system "$make $cmakeBuildArgs";

Renaming $make to $cmakeBuildCommand would make this read better:

	$result = system "$cmakeBuildCommand $cmakeBuildArgs";