[webkit-reviews] review denied: [Bug 37664] Chromium: Add --chromium option to new-run-webkit-websocketserver : [Attachment 55213] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Fumitoshi Ukai <ukai at chromium.org>'s
request for review:
Bug 37664: Chromium: Add --chromium option to new-run-webkit-websocketserver
https://bugs.webkit.org/show_bug.cgi?id=37664

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
+	 You can override this routine if necessary.

doesn't seem to add any clarity and should likely be removed.

Seems:
 201	     self._port_obj.setup_environ_for_server()

Should return an env.  Then you don't ever need to modify os.environ.

this should be some sort of helper assert:
124	    sys.platform = 'cygwin'
 125	    
self.assertTrue(isinstance(factory.get(options=self.chromium_options),
 126					chromium_win.ChromiumWinPort))
self.assert_port("cygwin", chromium.ChromiumWinPort, self.chromium_options)
or similar.

That would get rid of a bunch of copy/paste code in your test file.

r- for the environ modification and the copy/paste code.