[DISCUSS] Bot for stale PRs on GitHub

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Bot for stale PRs on GitHub

Ufuk Celebi-2
(1) I agree with Aljoscha's line of arguing here. A staleness bot is
quite the opposite of “sweeping things under the rug". A clear and
automated message about the state of a PR provides good value to
contributors, reviewers, and other people monitoring PRs. Asking
committers to proactively close PRs can be an uncomfortable personal
experience (at least that's how I usually felt when I suggested to
close a PR or actually closed it).

As Cameron pointed out, closed PRs don't disappear and the PR branches
are still available in case a PR is closed mistakenly.

Therefore, I'm +1 to try out the bot. (Let me clarify that I don't
expect this to magically solve all the problems that were pointed out
by Chesnay and others, but I believe that it's a step into the right
direction.)

(2) There's another discussion happening about adding a PR review bot
(see "[DISCUSS] Start new Review Process" thread on dev@). I'm not
sure whether we want to have to bots to interact with in the long
term. It might be worthwhile to sync these efforts.

– Ufuk

On Tue, Jan 15, 2019 at 10:40 AM Aljoscha Krettek <[hidden email]> wrote:

>
> @Chesnay I agree that we did not handle PRs well at all in the past and we need to change more things than adding a stale bot to address them. I think, however, that a state bot is one piece of the solution for this because it makes the staleness of PRs more apparent. Plus, it will clean up those PRs that really are stale beyond rescue and therefore provide us a better number of really open PRs.
>
> For me, the main point right now is that the bot will clean up old PRs (some of them from 2015) so that we get a more accurate number of open PRs. The bot will also tag state PRs with a “stale” label, which allows us to see at a glance (with a GitHub query) what PRs are stale.
>
> So far the two arguments I hear against adding a stale bot are:
>  - It adds noise
>  - Closing PRs is sweeping problems under the rug
>
> I think that an inaccurate number of PRs is more noise than a bot that reminds us (after say 2 months) about a PR. And the bot is actually the opposite of “sweeping things under the rug, before, stale PRs are being silently ignored and get forgotten. With a bot, there would be an automated system that makes sure we don’t silently sweep PRs under the rug.
>
> I see three groups of people that responded to this:
>  - people that work on projects that have a stale PR. those people are very enthusiastic about adding such a bot to Flink
>  - people that don’t work on projects that have a stale PR but are interested in trying it
>  - people that don’t work on projects that have a stale PR but are very much against against adding such a bot
>
> There is no people that work on a project that have a stale PR but think it’s not a good idea. In my mind this makes it quite clear that we should add such a bot.
>
> As I said above, I don’t think the stale PR is the complete solution but it’s part of the solution.
>
> > On 15. Jan 2019, at 02:28, Congxian Qiu <[hidden email]> wrote:
> >
> > I agree with Chesnay here.
> > How about introducing a bot just to tag the stale PRs, not close them. Then
> > we can get the numbers of how many stale PRs in there, and go farther
> > according to the numbers.
> >
> > Best,
> > Congxian
> >
> > Timo Walther <[hidden email]> 于2019年1月14日周一 下午10:30写道:
> >
> >> I totally agree with Chesnay here. A bot just treats the symptoms but
> >> not the cause.
> >>
> >> Maybe this needs no immediate action but we as committers should aim for
> >> a more honest communication. A lot of PRs have a reason for being stale
> >> but instead of communicating this reason we just don't touch them.
> >>
> >> But let's introduce a bot and see how it will affect the situation.
> >>
> >> Regards,
> >> Timo
> >>
> >>
> >> Am 14.01.19 um 15:19 schrieb Chesnay Schepler:
> >>> For reference, I'm still very much -1 on this.
> >>>
> >>> The short version is that auto-closing PRs hides symptoms that lead to
> >>> stale PRs in the first place.
> >>>
> >>> As an example, consider flink-ml. We have a fair amount of open PRs
> >>> targeted at this feature, that naturally this bot would close.
> >>> What are they stale? Because at the time no committer was interested
> >>> in them. Why are they still around? Because, despite seeing virtually
> >>> no development for over 2(!!!) years, we still haven't officially
> >>> declared flink-ml as dead. There is no note in the docs discouraging
> >>> contributors from working on it, all the JIRAs still exist, and
> >>> naturally the PRs were not closed because "maybe someone will look at
> >>> the soon.".
> >>>
> >>> We have this issue also in other areas, like gelly, storm, python,
> >>> streaming-python. WebUI PRs also routinely become stale. Is anyone
> >>> asking why or how we could prevent that? Hell no. But sweeping it
> >>> under a rug? /Sign me up/.
> >>>
> >>> Committers should proactively close PRs that will not be merged so
> >>> that we can properly communicate to the contributor why this happened,
> >>> reflect this decision in JIRA and possibly update the contribution
> >>> guide as a means of preventing such PRs from being opened again. This
> >>> also provides committers with a reference based on which they can
> >>> close future PRs.
> >>>
> >>> Recommending contributors to continuously update their PRs to prevent
> >>> them from being closed automatically is a terrible idea. This should
> >>> only be recommended if a committer has actually taken interest in the
> >>> PR and would be willing to review an updated version.
> >>> Anything else completely disrespects the contributor's time, on the
> >>> off-chance that they actually do so.
> >>>
> >>> On 14.01.2019 09:34, Aljoscha Krettek wrote:
> >>>> I think the automatic closing is an integral part, without it we
> >>>> would never close those stale PRs that we have lying around from 2015
> >>>> and 2016.
> >>>>
> >>>> I would suggest to set the staleness interval quite high, say 2
> >>>> months. Thus initially the bot would mainly close very old PRs and we
> >>>> shouldn’t even notice it on day-to-day PRs.
> >>>>
> >>>> It seems there is a larger consensus for adding the PR bot. By the
> >>>> way, keep in mind that we can always disable the bot again if we
> >>>> don’t like it.
> >>>>
> >>>>> On 14. Jan 2019, at 03:33, Kurt Young <[hidden email]> wrote:
> >>>>>
> >>>>> +1 to the bot, but -1 to the automatically closing PR behavior.
> >>>>>
> >>>>> Can we just use the bolt to detect and tag the PR with stale flag
> >>>>> and leave
> >>>>> the decision whether to close the PR to the author?
> >>>>>
> >>>>> Best,
> >>>>> Kurt
> >>>>>
> >>>>>
> >>>>> On Sun, Jan 13, 2019 at 11:49 PM Kostas Kloudas
> >>>>> <[hidden email]>
> >>>>> wrote:
> >>>>>
> >>>>>> +1 to try the bot.
> >>>>>>
> >>>>>> It may, at first, seem less empathetic than a solution that involves a
> >>>>>> human monitoring the PRs,
> >>>>>> but, in essence, having a PR stale for months (or even years) is at
> >>>>>> least
> >>>>>> as discouraging for a
> >>>>>> new contributor.
> >>>>>>
> >>>>>> Labels could further reduce the problem of noise, but I think that
> >>>>>> this
> >>>>>> "noise" is a necessary evil
> >>>>>> during the "transition period" of moving from the current situation
> >>>>>> to one
> >>>>>> with cleaner PR backlog.
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Kostas
> >>>>>>
> >>>>>> On Sun, Jan 13, 2019 at 1:02 PM Dominik Wosiński <[hidden email]>
> >>>>>> wrote:
> >>>>>>
> >>>>>>>> Hey,
> >>>>>>>>
> >>>>>>> I agree with Timo here that we should introduce labels that will
> >>>>>>> improve
> >>>>>>> communication for PRs. IMHO this will show what percentage of PRs is
> >>>>>> really
> >>>>>>> stale and not just abandoned due to the misunderstanding or other
> >>>>>>> communication issues.
> >>>>>>>
> >>>>>>> Best Regards,
> >>>>>>> Dom.
> >>>>>>>
> >>>>
> >>>
> >>>
> >>
> >>
> >
> > --
> > Best,
> > Congxian
>
12