[webkit-reviews] review denied: [Bug 68691] new-run-webkit-tests is locale dependent : [Attachment 115004] Patch to land

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 14 12:29:26 PST 2011


Eric Seidel <eric at webkit.org> has denied vanuan at gmail.com's request for review:
Bug 68691: new-run-webkit-tests is locale dependent
https://bugs.webkit.org/show_bug.cgi?id=68691

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

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


> Tools/ChangeLog:1
> +2011-11-14  Vanuan  <vanuan at gmail.com>

Normally we use our full names in ChangeLogs.

> Tools/ChangeLog:194
> +>>>>>>> .r100139

Conflict marker.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:660
> +	   """ This is a hack (tests shouldn't be locale dependent).
> +	       Works only in unix environments.
> +	   """

I would have made this a comment instead of a doc-string.  I also would have
used the keyword FIXME: at the beginning to it's easily searchable.

Something like this:
# FIXME: This is a hack.  Tests should not be locale dependent.  This only
works in unix environments.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:663
> +	   env['LANGUAGE']='en'
> +	   env['LC_MESSAGES']='en_US.utf8'
> +	   env['LANG']='en_US.UTF-8'

Also, this should go in setup_environ_for_server instead.


More information about the webkit-reviews mailing list