[webkit-reviews] review granted: [Bug 135551] [iOS] run-webkit-tests chokes on unterminated UTF-8 when writing a test result : [Attachment 237866] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted David Farler <dfarler at apple.com>'s
request for review:
Bug 135551: [iOS] run-webkit-tests chokes on unterminated UTF-8 when writing a
test result
https://bugs.webkit.org/show_bug.cgi?id=135551

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

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


> Tools/ChangeLog:10
> +	   unicode (encoded as utf-8) when writing to files, in the

Nit: unicode => Unicode

And

utf-8 => UTF-8

> Tools/ChangeLog:12
> +	   rare case that an NSError description will make it to
> +	   stderr.

For your consideration, I suggest that you add an example to this description
of when an NSError with Unicode characters can be emitted to the standard error
stream.

> Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m:97
> +	   [self didCrashWithMessage:nil];

I take it that there is little value to pass a string to -didCrashWithMessage
that describes the reason for the NSException?

> Tools/Scripts/webkitpy/common/system/filesystem.py:228
> +	       # FIXME: Just f.write(contents) when switching to Python 3

This comment isn't very useful because it doesn't explain "why" we can simplify
this code. I suspect that there are many other areas of our Python code that
can be simplified or will need to be updated to work with Python 3. If you
choose to keep this comment I suggest elaborating why we can simplify this code
in Python 3.