Re: Automation and other changes

[Carol Willing]

Donald Stufft wrote:





On Apr 2, 2017, at 9:37 AM, Carol Willing > wrote:



On Sunday, April 2, 2017 at 6:20:52 AM UTC-7, p.f.moore wrote:

On 2 April 2017 at 14:18, Donald Stufft  wrote:
> Ok cool. Let me play around with this some and see what I can
come up with.
> We can always adjust it if we decide the workflow isn’t working
for us :D

By the way, thanks for doing this in the first place - regardless of
whether what you do directly helps me, or alters how I personally
work, I'm really grateful that you're taking the time to think about
the problem.
Paul


Donald and Paul,

This is good stuff :-)

Going forward, adding a PULL_REQUEST_TEMPLATE.md would help clarify
the need for a News item.




I’ve done this in https://github.com/pypa/pip/pull/4415 dunno if the
wording is good or not though.



Looks great :D You are a good writer too.








I would also consider modifying the label `Needs merged or rebased`
to `Needs rebase or merge`. Perhaps a color difference between
contributor needed actions and reviewer needed actions would be
helpful for those viewing on GitHub.



I’ve done this as well.



Thanks!




—
Donald Stufft

Re: Automation and other changes

[Donald Stufft]

> On Apr 2, 2017, at 1:26 PM, Donald Stufft  wrote:
> 
> Sounds like between my starting to lean towards it, your preference, and the 
> fact it would trigger a new mail notification for Paul’s workflow too, that a 
> special command is the way to go. I will go ahead and implement that.


Ok! I have this implemented now, you can see it in action at 
https://github.com/pypa/pip/pull/4406 . 
I’m going to hold off on publishing docs because I want to switch to the user 
@pypa-bot instead of @BrownTruck but when I tried to create that user, GitHub 
flagged the account so it has to get manually reviewed first.

But basically, currently anyone can do “@BrownTruck request review” (must be on 
a line by itself, other stuff can be in the message though) and it will dismiss 
all of the current reviews, which moves it back into the “review queue” in the 
above work flow AND it triggers an email from the person to get email based 
workflow a notification that the PR is ready for more review.

Once I have it set up at @pypa-bot I’ll write the documentation for it, get it 
so that it comments about the command on the first request changes review (one 
per PR), and start sending periodic emails about things.

—
Donald Stufft

Re: Automation and other changes

[Xavier Fernandez]

First of all, thanks a lot for all the work you've put in pip, especially
lately.

Like Paul, I rely mostly on mail notifications to follow new/updated
issues/PR.
I usually read everything and if I don't respond immediately, I'm used to
leaving the mail as unread to - hopefully - come back to it later (it does
not work so well, since I've currently 238 unread mails in my Github/pip
label ;) ).
I'm not particularly satisfied with this workflow (even if it globally
works), so any improvement is very welcome.

Between the different options A (dismiss review on update), B (dismiss
review on contributor special message) and C (nothing), I think B has my
favor, (followed by A and C).

What I like with B is that is could be a first step for allowing more
control on the issue for the user (via Browntruck):
cf https://github.com/kubernetes/test-infra/blob/master/commands.md for
possible examples (so maybe allow to add a restricted list of labels).
Concerning the added complexity for contributors, PULL_REQUEST_TEMPLATE.md
could indeed be quite helpful.

The main issue is indeed that, when a contributor updates its PR and does
not post any message, nobody gets notified.
With option B, this is solved naturally since the user will have to post a
message, removing the "change needed" label but also triggering a new mail
notification :).

Concerning the failing tests, maybe Browntruck could also post a message to
tell the user that tests are broken (but I'm wondering if travis does not
already do that) if the tests are failing and the pull request has not been
updated since a day ?

Concerning the email digest, this seems like a good idea, weekly but not
daily.

Hopefully, with all this, we'll be able to merge PRs quicker which should
keep our list of open PRs fairly low and might help motivating new
contributors :)

Cheers,

-- 
Xavier

Re: Automation and other changes

[Paul Moore]

On 2 April 2017 at 14:03, Donald Stufft  wrote:
> I was just about to ask if you thought if a daily or weekly email to
> pypa-dev enumerating the contents of our review queue (where review queue ==
> the list of PRs waiting for action from a core dev) would be useful in a
> purely email based workflow like you’re using?

Depends how you define "waiting for action". A list of issues awaiting
review wouldn't help me so much, as a lot of open issues (Linux/Mac
specific ones) I don't have much to say on, and so ones I can comment
on would get lost. OTOH, a list of issues as you described (+1 review,
can be merged, tests passing) would be useful, as that would be
essentially a list of "just go in and push the merge button" items.
And I could easily grab a few minutes to go through any like that and
do a final check and merge.

Paul

Re: Automation and other changes

[Paul Moore]

On 2 April 2017 at 13:14, Donald Stufft  wrote:
> Another case of this that happens to me is I’ll get an email for the PR, see
> it, review it, think it’s a good idea, but the tests are still running (and
> they take 30+ minutes to run) so I’ll wait for that and end up forgetting
> about the PR as it sits there in a state that is 100% ready to be merged
> (it’s got a +1 review, tests are passing, everything is great) just because
> there isn’t an email to tell me.

Maybe a regular summary email listing PRs in this state would be a
useful reminder? I don't know if that's something that could be
automated.
Paul

Re: Automation and other changes

[Paul Moore]

On 2 April 2017 at 13:07, Donald Stufft  wrote:
> Thoughts?

Sounds reasonable - I didn't mean to say that I thought what you were
doing was wrong, just that the net effect on me was likely to be
minimal because I use a purely email-based workflow. And yes, you're
right that does mean that if I don't have time *right now* to respond
to an email, the PR behind it tends to fall off my radar.

Paul