[webkit-reviews] review denied: [Bug 63528] Complete functions in filesystem.py : [Attachment 99935] patch, reduced

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Roland Steiner
<rolandsteiner at chromium.org>'s request for review:
Bug 63528: Complete functions in filesystem.py
https://bugs.webkit.org/show_bug.cgi?id=63528

Attachment 99935: patch, reduced
https://bugs.webkit.org/attachment.cgi?id=99935&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99935&action=review

Mostly just some style nits.

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:-317
> -	   if path not in self.fs.files or not append:

Why is the check for 'path not in self.fs.files' no longer needed?

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:340
> +    def __init__(self, fs, path):
> +	   WritableBinaryFileObject.__init__(self, fs, path)

Nit: I don't think we need this, it's called implicitly.

> Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:149
> +	   unicode_text_string = u'Ūnĭcōde̽'

It might be safer to have this be unicode escaped (e.g., u'\uXXXX\uXXXX')
rather than trying to inline directly.	I think we expect all our python files
to be utf8.