[webkit-reviews] review denied: [Bug 127169] If not runned on MAC, "Tools/Scripts/build-webkit" should not post Safari related informations : [Attachment 221463] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has denied Éva Balázsfalvi
<balazsfalvi.eva at stud.u-szeged.hu>'s request for review:
Bug 127169: If not runned on MAC, "Tools/Scripts/build-webkit" should not post
Safari related informations
https://bugs.webkit.org/show_bug.cgi?id=127169

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=221463&action=review


> Tools/ChangeLog:3
> +	   If not runned on MAC, "Tools/Scripts/build-webkit" should not post
Safari related informations

Nit: MAC => Mac

Mac is not an acronym. It's shorthand for Macintosh.

> Tools/Scripts/build-webkit:431
> +    if ($launcherPath ne "0" && $launcherName ne "0") {

Without loss of generality, take $launcherName to be undefined (say
launcherName() falls off the end of the function) then evaluating the condition
of this if statement will cause an uninitialized value warning because we run
this script with -w (see the first line in this script). Moreover, it seems
weird to define "0" as an invalid value and compare to it. It's sufficient to
look at the truth value of the variables:

if ($launcherPath && $launcherName) {
   ...
}

> Tools/Scripts/webkitdirs.pm:1290
> +    } elsif (isDarwin()) {

Can you elaborate on this change?

> Tools/Scripts/webkitdirs.pm:1305
> +    } elsif (isDarwin()) {

Ditto.