[Matplotlib-devel] new github features

[Ryan May]

Hi,

In general, given the griping I've seen over time for how hard it is to do
code review on GitHub Pull Requests, I think we (the open source community)
owe it to GitHub to try out the new feature they rolled out to address and
give them feedback--that's the only way the feature can ever meet our needs.

For our (matplotlib) case, I think we get a few good gains:
- line comments are batched so we can cut down on the number of
notifications
- Your work is saved so you can come back later and finish--and adjust when
you see something later that invalidates your comment *before* commenting
for all to see
- Reviews are atomic items, so you can get an idea of what a specific
reviewer thinks (and not lose pieces)
- By having the state of Approved/Needs Changes actually tracked, there's
no need to read through the entire PR to see what objections are left.
(Granted, it does mean people have to come back and approve it once the
changes are made)

Seems like a lot to gain for little to no extra effort--having used it, I
certainly fail to see any problem. The matter of turning on required
reviews for protected branches is a whole other matter, though I will say
you can always let admins override.

Ryan


On Mon, Sep 19, 2016 at 8:24 PM, Thomas Caswell <tcaswell at gmail.com> 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.
>
> Has anyone looked at the 'projects' feature yet?
>
> 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.
>
> Tom
>
> _______________________________________________
> Matplotlib-devel mailing list
> Matplotlib-devel at python.org
> https://mail.python.org/mailman/listinfo/matplotlib-devel
>
>


-- 
Ryan May
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20160921/265347a3/attachment.html>