[python-committers] Vote to promote Pablo Salingo Salgado as core developer

[Victor Stinner]

2018-06-13 21:38 GMT+02:00 Serhiy Storchaka <storchaka at 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