2018-06-13 21:38 GMT+02:00 Serhiy Storchaka <storch...@gmail.com>: >> Pablo's merged PR: >> https://github.com/python/cpython/pulls?utf8=%E2%9C%93&q=is%3Aclosed+is%3Apr+author%3Apablogsal+ > > This includes closed without merging.
(sorry again, bad link.) > And reverted PRs. Adding ensure_disabled() (bpo-31356) was reverted because > the implementation caused crash and core developers have doubts about > usefulness of this feature at all. Hum, let me see: https://bugs.python.org/issue31356#msg301407 The feature has been proposed by Raymond Hettinger and approved ("+1") by Nick Coghlan. Pablo's PR has been merged by Raymond. It's not like Pablo proposed the idea himself and force to get this feature merged. Pablo just implemented an idea proposed by two other core developers. It happens that core developers disagree each others ;-) The feature has been quickly removed, it's not a big deal. > Adding posix_spawn() (bpo-20104) was > reverted in 3.7 because it was incomplete and had many bugs. It was left in > 3.8, and the work of completing and fixing it in process. (a) Two days after Pablo proposed his PR, he wrote an email to python-dev to get the widest audience to design posix_spawn() API: https://mail.python.org/pipermail/python-dev/2018-January/151661.html Gregory P. Smith merged Pablo's PR, after one month of review. IMHO Pablo did the best to get the correct API and a good implementation. (b) After the PR has been approved and merged, Serhiy noticed that the Python function doesn't expose one posix_spawn() feature. During Pycon US, we discussed what do to with posix_spawn() before the Python 3.7 release: urge to push a change, or revert. I proposed to Pablo to remove the feature from 3.7, get more time to stabilize the API and the implementation in master. He was fine with that. He could complain and ask to keep the feature (to get it released as soon as possible), but he didn't. I don't see anything wrong here. It's not uncommon that a newly merged feature is still discussed, and I don't see anything wrong with Pablo's behaviour here. I don't see how we could prevent such further discussions on posix_spawn(). At least, I don't see how Pablo could prevent these issues. > PR 4003 has long review history, the final merged version is very different > from the initial > one. "bpo-31786: Make select.poll.poll block if ms < 0 for every value of ms" https://github.com/python/cpython/pull/4003 Oh, I recall that one, I recall that *I* was super pedantic :-) You (Serhiy) asked to fix select.poll.poll() for negative timeout. This is exactly what Pablo did. His PR was fine on that aspect. Then I started to complain and ask to use my private _PyTime API. I also asked to implement a new rounding mode for _PyTime: a complex task, because it requires to modify a lot of files, write new tests, etc. This C code is not well documented. Then I asked to not only modify select.poll, but many other functions. The final PR is very different because I requested to modify much more files and fix many more functions. At the end, I really like the final commit: https://github.com/python/cpython/commit/2c15b29aea5d6b9c61aa42d2c24a07ff1edb4b46 It adds a new rounding mode (_PyTime_ROUND_UP), write a non-trivial function test for negative timeout, has a good NEWS entry, etc. Pablo showed that he is able to implement large change and fix all cases, not a single case. IMHO it's a good example, rather than a bad example :-) > Many of other PRs are documentation fixes. Is it an issue? > I think Pablo will be good core developer and agree with the description > given by Victor. But it seems that he still needs to learn something about > what changes are good for Python. Ok, I account your -1 vote. But I disagree you with :-) Pablo worked on complex changes, so I'm not surprised that his changes required a lot of discussion, and that there are some hiccups like posix_spawn() issues. By the way, posix_spawn() is one of the most complex C function that I know... Compare its API to dup() API: well, dup() is simpler :-) But posix_spawn() seems efficient and very useful. Again, I don't see anything wrong with Pablo's behaviour or the quality of his work. I understand that you want more reviewers on posixmodule.c changes, and there is where I expect that Pablo will help :-) In private, I told Pablo that I consider that the main difference between a core developer and a contributor is that core developers are expected to spend more time on reviews and mentoring. Pablo proved its steady involvment in Python for almost one year with multiple significant contributions (new os functions). IMHO you are pushing the bar too high. Victor _______________________________________________ python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers Code of Conduct: https://www.python.org/psf/codeofconduct/
