[webkit-reviews] review granted: [Bug 75931] webkitpy: add os_name, os_version to platforminfo : [Attachment 121912] revamp to make it more mockable, add is_mac(), etc.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 75931: webkitpy: add os_name, os_version to platforminfo
https://bugs.webkit.org/show_bug.cgi?id=75931

Attachment 121912: revamp to make it more mockable, add is_mac(), etc.
https://bugs.webkit.org/attachment.cgi?id=121912&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121912&action=review


Thanks for your work on this.

> Tools/Scripts/webkitpy/common/system/platforminfo.py:39
> +    Public (static) properties:
> +    -- os_name
> +    -- os_version

I'm slightly wary about making the public, now that you have the is_mac()
stuff.	We wish to discourage folks from writing error-prone string
comparisons.

> Tools/Scripts/webkitpy/common/system/platforminfo.py:67
> +	   if self.os_name == 'mac':

I think you want is_mac() here.