Management of PRs, issues, and threads

[Juan Nunez-Iglesias]

Hi everyone,

We've had a couple of community fails on GitHub recently:

https://github.com/scikit-image/scikit-image/pull/1474#issuecomment-241283056

https://github.com/scikit-image/scikit-image/issues/2080

(The last one is missing a presumably-deleted comment where someone outside the project recommended to a potential contributor to just fork the project!).

That's just the couple I've noticed, I'm sure there are lots more. We definitely have many, many abandoned PRs from >1y, >2y ago.

Obviously, something in our process isn't working. I don't have immediate solutions, but here's a couple of suggestions:

- we need a system to assign core devs to PRs/issues. Currently, it's too easy for all of us to go, "someone else on the team will handle this". We need a fair way to assign a manager to each issue/PR. The assigned dev wouldn't necessarily be responsible for review, but they would be responsible for chasing up other reviewers.

- NEVER close a PR without the explicit consent of the contributor, OR after the contributor has been non-responsive for e.g. 1mo and two pings. In this last situation I would even suggest that we need two core devs to sign off.

Other suggestions welcome in this thread. We should try to get a new process hammered out very soon.

I also want to acknowledge @soupault for his triage/labelling work, which is a massive step in the right direction. But we need to follow up his triage with some actual process.

Juan.

[Emmanuelle Gouillart]

Hi Juan,

thanks for bringing these issues to light. How about we tackle the queue
of open PRs from its two ends? That is, for the new PRs we have this
manager system (we still need to decide how to assign them), and at the
same time we go through old PRs to try to unblock or (worst case) close
them?

Best,
Emma

[Egor Panfilov]

Hi everyone!

Remember me hyping around with the GitHub bot idea?

Here it is:

- in the source code: https://github.com/facebook/mention-bot

- in action: https://github.com/tensorflow/tensorflow/pull/4239#issuecomment-245180925 

Just putting it here to bookmark. If someone of you will decide to play with it, I'm all ears to hear the feedback.

I'll most likely give it a try in the coming weeks.

Regards,

Egor

вторник, 23 августа 2016 г., 0:26:52 UTC+3 пользователь Emmanuelle Gouillart написал:

[Stefan van der Walt]

On Thu, Sep 8, 2016, at 07:22, Egor Panfilov wrote:

Remember me hyping around with the GitHub bot idea?

Here it is:

- in the source code: https://github.com/facebook/mention-bot

- in action: https://github.com/tensorflow/tensorflow/pull/4239#issuecomment-245180925 

This is cute.  Although I think what could be even more useful is a feature I saw elsewhere: instead of clicking the merge button, you instruct it to re-run the test suite and merge upon successful completion.  This way your PRs are always checked against latest master, not whatever was around when they were first made.

Stéfan

[Matthew Brett]

Hi,

Yes, this was the homu bot - we have a copy of that running on the
nipy server in Berkeley. Matthias B of IPython fame was talking
about sharing some work to refactor the homu code into something more
general to extend the range of github tasks it could do - maybe
including mention-automation like the face-book-bot.

Matthew

[Egor Panfilov]

Dears,

I believe there might be a generic solution already: see my comment (https://github.com/scikit-image/scikit-image/issues/2269#issuecomment-243357010) and, particularly, "rust-lang" reference.

Regards,

Egor

--
You received this message because you are subscribed to the Google Groups "scikit-image" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scikit-image+unsubscribe@googlegroups.com.
To post to this group, send an email to scikit...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/scikit-image/CAH6Pt5phve%3DS1t1haAVT%3D3%3DRhwwDBdZzO9Mqh1ED4_cDx2B-Pw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Juan Nunez-Iglesias]

> This way your PRs are always checked against the latest master, not whatever was around when they were first made

I thought Travis did this already?

https://docs.travis-ci.com/user/pull-requests

"Rather than test the commits that have been pushed to the branch the pull request is from, we test the merge between the origin and the upstream branch."

However this would imply that Travis wouldn't do anything when you can't automatically merge. I don't remember whether this is True...

--
You received this message because you are subscribed to the Google Groups "scikit-image" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scikit-image...@googlegroups.com.

[Nelle Varoquaux]

Hi scikit-image folks!

> We've had a couple of community fails on GitHub recently:
> https://github.com/scikit-image/scikit-image/pull/1474#issuecomment-241283056
> https://github.com/scikit-image/scikit-image/issues/2080
> (The last one is missing a presumably-deleted comment where someone outside
> the project recommended to a potential contributor to just fork the
> project!).

Being the person that reported one of those community fails, I'd like
to say that I've had great experience with the scikit-image community
overall. Contributors are (usually) very well welcomed, feedback is
friendly, and we can have discussion and debates about "hot" topics
(like dropping python2.7) in a friendly and nice environment. I've
recently contributed to another scientific python project where many
members of the community were aggressive in their reviews, without
giving valuable feedback on PRs, so I value scikit-image's (&
scikit-learn) community even more.

> That's just the couple I've noticed, I'm sure there are lots more. We
> definitely have many, many abandoned PRs from >1y, >2y ago.
>
> Obviously, something in our process isn't working. I don't have immediate
> solutions, but here's a couple of suggestions:
>
> - we need a system to assign core devs to PRs/issues. Currently, it's too
> easy for all of us to go, "someone else on the team will handle this". We
> need a fair way to assign a manager to each issue/PR. The assigned dev
> wouldn't necessarily be responsible for review, but they would be
> responsible for chasing up other reviewers.
>
> - NEVER close a PR without the explicit consent of the contributor, OR after
> the contributor has been non-responsive for e.g. 1mo and two pings. In this
> last situation I would even suggest that we need two core devs to sign off.

I would not close old PR, for the sake of closing them. If the
contributor is not interested in finishing the contribution and the
patch is almost ready, it might be a good way to introduce new
contributors (/students) to the PR. This can probably be done by an
"Need contributor" tag + "Easy fix", though I haven't thought more
about this

> Other suggestions welcome in this thread. We should try to get a new process
> hammered out very soon.
>
> I also want to acknowledge @soupault for his triage/labelling work, which is
> a massive step in the right direction. But we need to follow up his triage
> with some actual process.
>
> Juan.
>

> --
> You received this message because you are subscribed to the Google Groups
> "scikit-image" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to scikit-image...@googlegroups.com.

> To post to this group, send email to scikit...@googlegroups.com.

> To view this discussion on the web, visit

> https://groups.google.com/d/msgid/scikit-image/CA%2BJHcKSh-k%2ByM0yBTRzTy%3DZgY7%2BOoEmPkKNA4xUgudeyhCJ5Dw%40mail.gmail.com.

[Johannes Schönberger]

This should very be interesting in this regard: https://github.com/blog/2256-a-whole-new-github-universe-announcing-new-tools-forums-and-features

To post to this group, send an email to scikit...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/scikit-image/CAE-UAvSnKid2zn9bM4%3DdU2mvG96jDg8%2BfTayEOw%3DTA%2BS4u27zg%40mail.gmail.com.

[Stefan van der Walt]

On Sun, Sep 11, 2016, at 23:13, Juan Nunez-Iglesias wrote:

> This way your PRs are always checked against the latest master, not whatever was around when they were first made

I thought Travis did this already?

https://docs.travis-ci.com/user/pull-requests

"Rather than test the commits that have been pushed to the branch the pull request is from, we test the merge between the origin and the upstream branch."

This happens the moment you make the PR, or push new commits.  Ideally, you want to run the tests just before merging.

Stéfan