[webkit-reviews] review granted: [Bug 116339] Add --sdk flag to jsDriver.pl to allow running in the iOS simulator : [Attachment 202743] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted David Farler
<dfarler at apple.com>'s request for review:
Bug 116339: Add --sdk flag to jsDriver.pl to allow running in the iOS simulator
https://bugs.webkit.org/show_bug.cgi?id=116339

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

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


r=me with some minor fixes to avoid warnings and unnecessary checks.

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:180
> +		   chomp($shell_command = `xcrun -sdk $opt_sim_sdk -find sim`)
if $opt_sim_sdk;

You don't need the " if $opt_sim_sdk;" here now that it's inside the if block.

> Source/JavaScriptCore/tests/mozilla/jsDriver.pl:184
> +	       $shell_command .= " $opt_arch ";

If $opt_sim_sdk is not set, we now have a space at the beginning of
$shell_command, and I think $shell_command will be undef here (so we'll get a
warning about appending a string to undef).

You might consider just adding an else block to the if ($opt_sim_sdk) above to
initialize $shell_command to the empty string:

    } else {
	$shell_command = "";
    }

Then you can change this line to:

    $shell_command .= "$opt_arch ";