<div dir="ltr">Hi Victor,<br><br>I have reviewed the PEP and I think it is good. Thank you so much for pushing this topic and for your very thorough review of all the feedback, related issues and so on. It is an exemplary PEP!<br>

<br>I've made a bunch of small edits (mostly to improve grammar slightly, hope you don't mind) and committed these to the repo. I've also got a few more comments on the text that I didn't want to commit behind your back; I've written these up in a Rietveld review of the PEP (which you can also use to see exactly what I did already commit). <a href="https://codereview.appspot.com/13240043/" target="_blank">https://codereview.appspot.com/13240043/</a><br>


<br>Here's a summary of those review changes:<br><br><pre name="cl-message-0"><a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt</a>
File pep-0446.txt (right):

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode25" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode25</a>
pep-0446.txt:25: descriptors.
I'd add at this point:

We are aware of the code breakage this is likely to cause, and doing it anyway
for the good of mankind. (Details in the section "Backward Compatibility"
below.)

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode80" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode80</a>
pep-0446.txt:80: inheritable handles are inherited by the child process.
Maybe mention here that this also affects the subprocess module?  (You mention
it later, but it's important to realize at this point.)

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode437" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode437</a>
pep-0446.txt:437: condition.
As C-F Natali pointed out, this is not actually a problem, because after fork()
only the main thread survives.  Maybe just delete this paragraph?

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode450" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode450</a>
pep-0446.txt:450: parameter is a non-empty list of file descriptors.
Well, it could pass closefrom() the max of the given list and manually close the
rest. This would be useful if the system max is large but none of the FDs given
in the list is. (This would be more complex code but it would address the issue
for most programs.)

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode486" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode486</a>
pep-0446.txt:486: * ``socket.socketpair()``
I would call out that dup2() is intentionally not in this list, and add a
rationale for that omission below.

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode528" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode528</a>
pep-0446.txt:528: by default, but non-inheritable if *inheritable* is ``False``.
This might be a good place to explain the rationale for this exception.

<a href="https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode538" rel="nofollow">https://codereview.appspot.com/13240043/diff/3001/pep-0446.txt#newcode538</a>
pep-0446.txt:538: descriptors).
I would say it should not be changed because the default is still better. :-)</pre><br>-- <br>--Guido van Rossum (<a href="http://python.org/~guido" target="_blank">python.org/~guido</a>)</div>