[webkit-reviews] review denied: [Bug 37518] new-run-webkit-websocketserver failed to launch on Win : [Attachment 54042] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 22 12:28:59 PDT 2010


Adam Barth <abarth at webkit.org> has denied Fumitoshi Ukai <ukai at chromium.org>'s
request for review:
Bug 37518: new-run-webkit-websocketserver failed to launch on Win
https://bugs.webkit.org/show_bug.cgi?id=37518

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
WebKitTools/Scripts/webkitpy/layout_tests/port/server_process.py:46
 +	from win32pipe import PeekNamedPipe
Can we abstract this functionality into some kind of library instead of having
these ifs all over the place?

WebKitTools/Scripts/webkitpy/layout_tests/port/server_process.py:83
 +		fd = self._proc.stdout.fileno()
Please use more verbose variable names.  I have no idea what this code is
doing.

WebKitTools/Scripts/webkitpy/layout_tests/port/server_process.py:206
 +	def _select(self, rlist, wlist, xlist, timeout):
Again, these argument names are incomprehensible.

WebKitTools/Scripts/webkitpy/layout_tests/port/server_process.py:222
 +		time.sleep(0.1)
Do we really need this sleep loop?  This seems janky.


More information about the webkit-reviews mailing list