[issue27584] New addition of vSockets to the python socket module

R. David Murray report at bugs.python.org
Sat Dec 17 22:31:32 EST 2016


R. David Murray added the comment:

The second patch seems to be missing the configure changes.  Also, the tests have some over-long lines (we limit line lengths to 79 characters).  I realize there are other long lines in that file, but no need to add more :)

There is trailing whitespace on a number of lines in your patch.

Since this is new, we may not want to accept it until the support hits upstream.  Specifically, it will be difficult to get a review if the reviewer has to build a custom kernel to test the code :)  You do say that the VMCI is upstream, but I don't know what that means.  Which upstream?

Note: I'm not familiar with the socket C code, so I haven't reviewed the C code changes.  The tests look fine to me.

For the docs, the proposal doesn't seem to follow the format of the existing docs.  I would expect only the first paragraph located where you have it.  The remaining constants should be in the 'module contents'/'constants' section, I think.  Yes, that means each one gets a '.. versionadded' label.  Presumably also an 'availablility' label with whatever the minimum kernel version is...another reason we may need to wait.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue27584>
_______________________________________


More information about the Python-bugs-list mailing list