[python-committers] My (positive) feedback on the new CPython workflow

[Terry Reedy]

On 7/18/2017 2:31 PM, Brett Cannon wrote:

> Once again, glad the goals are panning out. :) Key thing has always been 
> to increase our bandwidth and part of that was always to push more on to 
> contributors so they can be more self-servicing.
> 
> 
>     * most contributors create backports (using cherry-pick) themself.
>     Before, this painful/error-prone task was usually done by a single
>     developer without any review, without pre-commit tests, etc.

Backports affecting the same file should be done in the same order, with 
the same 'before' state.

   Real example: I merged PR A for file x.py and updated my clone to the 
result of the merge.  I then prepared and merged PR B for the same file. 
  Someone 'helpfully' prepared a backport of PR B, call it PR B3.6.  It 
passed CI but was wrong.  Fortunately, I checked the 'after' state of 
the file on a side-by-side diff. I prepared and merged PR A3.6, updated 
the 3.6 branck of my clone, and then prepared a new and correct backport 
of PR B.

Correct backporting is most easily assured if backports are done 
immediately.  I currently do so myself instead of requesting and waiting 
for a contributor to do so (who likely is not immediately available). 
Even better would be for the backport to happen automatically.


> My wishlist that I don't think anyone is actively working on ATM is:

 >  2. Bot to backport PRs (which could also be automatically merged)

So, to me, this is the priority item on the list.

>  1. Bot to merge approved PRs (e.g. Rust's Homu)

How is a bot going to re-write the commit message?  Beside which, 
'approve' does not necessarily mean 'commit now'.

>  3. Automate Misc/ACKS (or do away with it and do something like
>     https://thanks.rust-lang.org/)

Nice, but it is easy enough to ask the new contributor to do it ;-).

>  4. Make test coverage reports consistent

First, we need to make sure that the only difference in the before and 
after code is the diff shown on the PR files pages, so that the coverage 
difference is only due to the actual patch.  From what I have seen, this 
does not seem true now.  One way to make this happen would be to update 
the PR (or a copy thereof) to current repository, so that the only files 
affected by the tested PR are the ones intended and visible.

Second, at least for IDLE, the .coveragerc file needs a few exclude 
lines added.  I suspect that they would be compatible with the rest of 
the tests.  (This could be checked with grep.)

>  5. Automatically close stale PRs (e.g. not signing CLA, changes
>     requested but not being made, etc.)

What does 'close' (without merging) mean?  I would not want older (IDLE) 
PRs permanently gone in any sense just because I am busy reviewing other 
patches.  I recently merged PR based on a patch uploaded to BPO 8 years ago.

Not signing CLA is a special case.

>  6. Bot to nudge core devs when they forget something (e.g. post-merge,
>     a comment if someone forgot to change the commit message or PR
>     number to have a "GH-" prefix)

When possible, I would prefer to get reminders before the merge.

--
Terry Jan Reedy