[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