[Matplotlib-devel] new github features

[Eric Firing]

On 2016/09/19 4:24 PM, Thomas Caswell wrote:
> Folks,
>
> Have people had a chance to look at the new review tools on github?  Do
> we want to work these into our workflow?  It looks like we can set it so
> that PRs can only be merged via the web UI with at least one approve
> review and no 'needs work' reviews if we want.   My initial reaction is
> we should try to use the review tools, but hold on on the engineering
> control on merges until we have a better sense of what that would look like.

I'm not a fan of this new mechanism.  One of the problems is that 
sometimes one doesn't know at the outset whether one will be doing a 
thorough review, or just starting to look and make a few comments.  I 
don't much like the automated comments it generates based on the review 
checkbutton, either.  Making this mechanism mandatory would be quite an 
irritant for little or no benefit, I think.

Some automated operations are good.  For example, if a "closes #1234" in 
an obvious location in a PR causes #1234 to be closed when the PR is 
merged, that's good.  Unfortunately, I haven't figured out how this 
works--it seems like sometimes it does, sometimes it doesn't. 
Presumably I just don't know the rule github is using.  So here, a 
better design would be a box in the PR specifically for linking in 
"closes" issue numbers.

Thumbs-up emoji counts are not helpful; I need to look to see whose 
thumb it is, and then I'm still left wondering exactly what was being 
approved.  And was it a thoughtful approval, or just a twitch?

So overall, with my usual curmudgeon's hat on, covering my small brain, 
I favor keeping the automation simple, obvious, and fairly minimal.  For 
everything else, let's use normal human communications, not bots.

>
> Has anyone looked at the 'projects' feature yet?
Haven't looked.  Should I?
>
> A suggestion from Nelle Varoquaux is to follow skimage/sklearn and on
> review add a [mrg+N] to the title to help other devs find PRs that are
> almost ready to merge, but need another set of eyes.

Aha! So that's what those are!  The "N" is the number of eye-sets needed?
I don't object to having a few conventions like that.  If it ends up 
helping us cut down the open PR and issue counts, that would be great.
Conventions are better than bots.

Eric