On Tue, 18 Jul 2017 at 03:24 Victor Stinner <victor.stinner@gmail.com> wrote:
Hi,

2017-07-18 11:36 GMT+02:00 Antoine Pitrou <antoine@python.org>:
> Can I take the opportunity to say thank you again (both you and Larry)
> for the "blurb" tool?  It really makes an important difference when
> contributing.

Glad it's working out! And a thanks from me to everyone who participated in the discussions that led to blurb (and of course Larry for coding it up and putting up with me throughout the whole process :) .
 
>
> Regards
>
> Antoine.

I concur with Antoine, I'm now *very* happy with the new workflow. The
code became better, many things are much simpler for me (especially
the tasks that I don't have to do myself anymore ;-)), my productivity
increases and I see more and more active contributors, much more than
before the move to GitHub and new workflow!

Great! That was the goal the whole time so I'm glad it's starting to pay off.
 


== Introduction ==

Two friends asked me recently how is the new Python workflow going.

Somehow, I decided to not express myself on the topic to avoid dummy
knee-jerk reaction like "it sucks, it was better before, xxx doesn't
work, now I have to xxx, etc."

It took me at least 3 months to be used to the new workflow, while
tools were developed and enhanced, and the workflow was adjusted based
on our early feedback.

So, here is my feedback :-)


== pre-commit CI: less regressions ==

>From a pure technical point of view, IMHO pre-commit CI on Linux
(Travis CI) and Windows (AppVeyor) is a *MAJOR* enhancement. (I don't
count macOS, since it's not mandatory yet.)

For example, before GitHub, it was common to break Windows since too
few core developers were running tests *manually* on Windows.

It's hard to prove my point, so try to trust me: we reduce a lot the
number of regressions introduced by new changesets. Before, every two
weeks, we had to rush to push fixes in urgency (which sometimes leaded
to bad code which required multiple iterations until we find the best
fix).

It was especially annoying when a dev pushed a big change just before
being away for 8-12 hours, which means having to push a fix without
the review of the change author :-(

I have to admit I also appreciate the CI testing a lot since I worry a lot less about whether Python is in some broken state. And then thanks to all the hard work you've done, Victor, we know that if the buildbots go red that we have actually regressed and need to roll back a change instead of going "that buildbot is just always broken so you can ignore it".

It's getting to the point that I'm toying with the idea of making a Google Assistant command for my Google Home so I can say, "Hey Google, how is CPython doing?" and have it tell me if the buildbots are all green. :) And I still dream about having some LED light or grid for sprints where we can visibly see the buildbot statuses to know quickly when something has broken. But none of that was possible previously, so it's really exciting to have reached a stability point where this isn't a stupid idea. :)
 


== More contributors, more contributions, faster reviewed/merged ==

In term of contributions, I looked at statistics yesterday and it
became clear the number of different authors is significantely much
higher, around 25 contributors / month before, now closer to 50:
https://www.openhub.net/p/python/contributors/summary

Well, Git allows to store the original author, so statistics are
simplify higher just because previously the tools failed to identify
the real author.

I'm going to be interested to see how the numbers look in Feb 2018 compared to now and see if there's a trend since the transition.
 

But I'm watching the bug tracker, pull requests, and Git commits:
there are a lot of *new* contributors, we get more contributions, and
we are *faster* to integrate them.

I also saw again inactive core contributors starting to review again,
or even write new PRs! It's a very good sign of the good health of our
project!

As Serhiy pointed out, though, some people have not made the transition. To help with this I plan on offering to do video chats with any core devs who want a personal walkthrough of the workflow, but I'm holding off on that until all of the critical workflow changes have landed as I don't want to be talking with someone and saying, "but this will be changing in a month or so". :)
 

More generally, the whole development seems to be more active, more
productive and more *healthy*.


== Better scale horizontally ==

The new workflow better scales horizontally since we delegated much
more work to the contributor. Previously the core developers were more
the bottleneck.

Tasks delegated to the contributors:

* create the PR itself (can you still remind when we had to download a
patch file and create a commit? ;-)),

* write the commit message (I always hated to do that without any kind
of review...),

* add the NEWS entry (before it was common to explicitly ask to *not*
write it... to avoid conflicts with Misc/NEWS...).

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.

Nice! Maybe we should have a message from Bedevere after a PR gets merged saying something like, "thanks for the PR and congrats on getting it merged! If you're interested in helping to backport your PR to older versions of Python, let us know and follow the instructions at XXX to create backporting PRs."
 

Note: I didn't see any contributor complaining about having to do
these tasks. It seems like it's more the opposite, they are happy to
be more *involved* in the project!


== GitHub ==

I never had to explain how to create an account on GitHub, nor how to
create a PR.

Signing the CLA doesn't seem to be a blocker point neither,
contributors are able to sign it and the bot tracks correctly the CLA
status (thanks Brett for the bot! it's nice to feel safe for legal
issues ;-)).

Yeah, I sleep a little better at night thanks to that thing. :)
 

Maybe we have an excellent documentation. Maybe developers already
know well the GitHub platform.

Even for backports, I didn't have to explain how to cherry-pick a
commit, but it was common that I pointed to the devguide section when
I proposed the author to do the backport himself/herself.

Having explicitly a list of "Approve" and "Requet changes" votes helps
to *quickly* have an overview of the review status.

I'm just not unconfortable with the fact that an approval is kept even
if the PR is modified after the review :-/ I would expect a list a
notice "changed modified after the review" or something like that. At
least, for my own reviews, to remind me to review again.

That wouldn't be too hard to add. I'm currently working on https://github.com/python/core-workflow/issues/94 which is going to label PRs based on their current stage (much like the stage field on bugs.python.org but much more automated). One of the labels is going to be "awaiting change review" for when a contributor has addressed requested changes, but it could be applied when the latest review is an approval but a post-approval change was made (along w/ a comment to point out that's why the label was added). Does that sound like what you're after?
 

The [First time contributor] label on PR helps to remind Misc/ACKS:
ask the author to add himself/herself to ACKS.

That's a great trick! I really wish we could programmatically access that more easily than scouring the git repo for committer details.
 

Compared to Rietveld, GitHub review tool is as "a good" (not much
better, not much worse). Sometimes, I'm lost in reviews: my comments
are hidden, I have to unfold many widgets. But it's not that bad. It
seems like avoiding rebase works around some of these issues.


== Git branches ==

Having to create a local Git branch, push it to my repository and
create PR is more work than attaching a patch file to bugs.python.org.
It requires to be more organized to track my "local" Git branches.

I'm now trying to remove a local branch after pushing the PR, since it
also exists in my "haypo" remote anyway. It helps to have a shorter
list of branches.

I also wrote a shell script to run "git fetch haypo --prune" to remove
"merged" branches (technically, I have to click on a button to remove
my branch, after I merge my own PR).

Sometimes, I also see branches in the upstream repository
(python/cpython), but it seems that we have less and less of them.

I think people are pretty good about cleaning up after themselves since those branches usually only come up when someone makes a change through the web UI.
 


== NEWS.d and blurb ==

Thank you Larry Hastings, Brett Cannon and others who helped to write
and integrate blurb: no more Misc/NEWS conflict, it's just amazing :-)
We *all* wanted that for *years*!

Just to keep people up-to-date with what I personally have planned or I know people are working on:
  1. Status check for a news entry (https://github.com/python/bedevere/pull/22)
  2. Add a link to the end of the PR body back to bugs.python.org (thanks to Kushal; https://github.com/python/bedevere/issues/3 and https://github.com/python/bedevere/pull/24)
  3. Label PRs based on what stage they are at (https://github.com/python/core-workflow/issues/94; started work on this)
  4. Make the CLA bot a status check instead of a label to make it a required status check (https://github.com/python/the-knights-who-say-ni/issues/42)
  5. Commit emails w/ diffs (thanks to Berker; https://github.com/python/bedevere/pull/27)

I think #2 and #5 cover the last initial complains people had about the new workflow, #1 and #3 will make things even better, and #4 is so I can sleep even better at night. ;)

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

  1. Bot to merge approved PRs (e.g. Rust's Homu)
  2. Bot to backport PRs (which could also be automatically merged)
  3. Automate Misc/ACKS (or do away with it and do something like https://thanks.rust-lang.org/)
  4. Make test coverage reports consistent
  5. Automatically close stale PRs (e.g. not signing CLA, changes requested but not being made, etc.)
  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)

I think that wishlist would get us as automated as reasonably possible while reminding people when the manual parts are accidentally done wrong.