You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Kenneth Knowles <kl...@google.com> on 2018/06/01 03:07:29 UTC
Re: Closing (automatically?) inactive pull requests
Update: you brought the information needed, and it is now enabled. Thanks
for the follow-through!
Since you dug into probot's details, I took the liberty of assigning
BEAM-4423 to you, in case throwing together the needed configs is fresh in
your mind and you are in the mood to continue. (if not, pass the hot
potato, back to me is OK too :-)
Kenn
On Thu, May 31, 2018 at 4:50 PM Alan Myrvold <am...@google.com> wrote:
> INFRA-16589 got closed asking to clarify that the probot-stale app would
> not have permissions to merge automatically.
> From my reading of the permissions documentation, it would not. I added a
> comment to INFRA-16589
>
> On Tue, May 29, 2018 at 10:05 AM Lukasz Cwik <lc...@google.com> wrote:
>
>> I opened up INFRA-16589
>> <https://issues.apache.org/jira/browse/INFRA-16589> for Apache infra to
>> install Stale but they denied a similar request INFRA-16394
>> <https://issues.apache.org/jira/browse/INFRA-16394> because of
>> permissions issues. I tried clarifying the permissions requested.
>>
>> On Tue, May 29, 2018 at 9:39 AM Scott Wegner <sw...@google.com> wrote:
>>
>>> +1. I've opened BEAM-4423 to capture the discussion here:
>>> https://issues.apache.org/jira/browse/BEAM-4423
>>>
>>> On Thu, May 24, 2018 at 5:34 PM Chamikara Jayalath <ch...@google.com>
>>> wrote:
>>>
>>>> +1 for automatically closing. Currently, contribution guide mentions
>>>> that pull requests without responses to actionable comments become stale
>>>> (and closed) after two months but last I checked there were many pull
>>>> requests that met this criteria but had not been closed after many months.
>>>> So seems like humans are reluctant to act on the established policy :)
>>>>
>>>> - Cham
>>>>
>>>> On Wed, May 23, 2018 at 11:25 AM Kenneth Knowles <kl...@google.com>
>>>> wrote:
>>>>
>>>>> That makes sense, to just focus on Beam's decision. It seems the tool
>>>>> is already built. I thought we just had to deploy it, but maybe not even
>>>>> that, if we can just activate it: https://github.com/apps/stale
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Wed, May 23, 2018 at 9:31 AM Ismaël Mejía <ie...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Given that reaching consensus in both communities seems like a harder
>>>>>> task
>>>>>> than just deciding our policy. in the Beam side Why don't we just go
>>>>>> ahead
>>>>>> and vote around this + build the tool, and if the Flink guys are
>>>>>> interested
>>>>>> they can take it, no?
>>>>>>
>>>>>> in the future we can share that code.
>>>>>> On Wed, May 16, 2018 at 3:31 PM Piotr Nowojski <
>>>>>> piotr@data-artisans.com>
>>>>>> wrote:
>>>>>>
>>>>>> > The question is what would such tool offer on top of over a
>>>>>> Github’s view
>>>>>> of PR sorted by “least recently updated”:
>>>>>>
>>>>>>
>>>>>>
>>>>>> https://github.com/apache/flink/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc
>>>>>> <
>>>>>> https://github.com/apache/flink/pulls?q=is:pr+is:open+sort:updated-asc
>>>>>> >
>>>>>>
>>>>>> > ? Maybe it would be good enough to have a policy about waiting x
>>>>>> months/weeks for a contributor to respond. If he doesn’t, we either
>>>>>> take
>>>>>> over PR we or close it. Having “clean” view of oldest PRs would be
>>>>>> beneficial for reviewing PRs in ~FIFO order as part of community work.
>>>>>>
>>>>>> > Having
>>>>>>
>>>>>> > > On 16 May 2018, at 10:57, Fabian Hueske <fh...@gmail.com>
>>>>>> wrote:
>>>>>> > >
>>>>>> > > Hi,
>>>>>> > >
>>>>>> > > I'm not objecting closing stale PRs.
>>>>>> > > We have quite a few PRs with very little chance of being merged
>>>>>> and I
>>>>>> would
>>>>>> > > certainly appreciate cleaning up those.
>>>>>> > > However, I think we should not automate closing PRs for the
>>>>>> reasons I
>>>>>> gave
>>>>>> > > before.
>>>>>> > >
>>>>>> > > A tool that reminds us of state PRs as proposed by Till seems
>>>>>> like a
>>>>>> good
>>>>>> > > idea and might actually help to re-adjust priorities from time to
>>>>>> time.
>>>>>> > >
>>>>>> > > @Yazdan: Right now there is no assignment happening. Committers
>>>>>> decide
>>>>>> > > which PRs they want to review and merge.
>>>>>> > >
>>>>>> > > Best, Fabian
>>>>>> > >
>>>>>> > > 2018-05-16 4:26 GMT+02:00 Yaz Sh <ya...@gmail.com>:
>>>>>> > >
>>>>>> > >> I have questions in this regard (you guys might have addressed
>>>>>> it in
>>>>>> this
>>>>>> > >> email chain):
>>>>>> > >> how PRs get assigned to a reviewer apart of a contributor tag
>>>>>> someone?
>>>>>> > >> what if PR never gets a reviewer attention and it became
>>>>>> in-active due
>>>>>> to
>>>>>> > >> long review respond? should Bot assign a reviewer to a PR based
>>>>>> on
>>>>>> > >> reviewers interest (i.e., defined via tags) and notify he/she if
>>>>>> PR is
>>>>>> > >> waiting for review?
>>>>>> > >>
>>>>>> > >>
>>>>>> > >> Cheers,
>>>>>> > >> /Yazdan
>>>>>> > >>
>>>>>> > >>
>>>>>> > >>> On May 15, 2018, at 12:06 PM, Thomas Weise <th...@apache.org>
>>>>>> wrote:
>>>>>> > >>>
>>>>>> > >>> I like Till's proposal to notify the participants on the PR to
>>>>>> PTAL.
>>>>>> But
>>>>>> > >> I
>>>>>> > >>> would also suggest to auto-close when no action is taken, with a
>>>>>> friendly
>>>>>> > >>> reminder that PRs can be reopened anytime.
>>>>>> > >>>
>>>>>> > >>> The current situation with 350 open PRs may send a signal to
>>>>>> contributors
>>>>>> > >>> that it may actually be too much hassle to get a change
>>>>>> committed in
>>>>>> > >> Flink.
>>>>>> > >>> Since the count keeps on rising, this is also not a past
>>>>>> problem.
>>>>>> Pruning
>>>>>> > >>> inactive PRs may help, that will also give a more accurate
>>>>>> picture of
>>>>>> the
>>>>>> > >>> lack of review bandwidth, if that is the root cause.
>>>>>> > >>>
>>>>>> > >>> Thanks,
>>>>>> > >>> Thomas
>>>>>> > >>>
>>>>>> > >>>
>>>>>> > >>>
>>>>>> > >>>
>>>>>> > >>>
>>>>>> > >>> On Tue, May 15, 2018 at 8:24 AM, Ted Yu <yu...@gmail.com>
>>>>>> wrote:
>>>>>> > >>>
>>>>>> > >>>> How does the bot decide whether the PR is waiting for reviews
>>>>>> or is
>>>>>> > >> being
>>>>>> > >>>> abandoned by contributor ?
>>>>>> > >>>>
>>>>>> > >>>> How about letting the bot count the number of times
>>>>>> contributor pings
>>>>>> > >>>> committer(s) for review ?
>>>>>> > >>>> When unanswered ping count crosses some threshold, say 3, the
>>>>>> bot
>>>>>> > >> publishes
>>>>>> > >>>> the JIRA and PR somewhere.
>>>>>> > >>>>
>>>>>> > >>>> Cheers
>>>>>> > >>>>
>>>>>> > >>>> On Tue, May 15, 2018 at 8:19 AM, Till Rohrmann <
>>>>>> trohrmann@apache.org>
>>>>>> > >>>> wrote:
>>>>>> > >>>>
>>>>>> > >>>>> I'm a bit torn here because I can see the pros and cons for
>>>>>> both
>>>>>> sides.
>>>>>> > >>>>>
>>>>>> > >>>>> Maybe a compromise could be to not have a closing but a
>>>>>> monitoring
>>>>>> bot
>>>>>> > >>>>> which notifies us about inactive PRs. This could then trigger
>>>>>> an
>>>>>> > >>>>> investigation of the underlying problem and ultimately lead
>>>>>> to a
>>>>>> > >>>> conscious
>>>>>> > >>>>> decision to close or keep the PR open. As such it is not
>>>>>> strictly
>>>>>> > >>>> necessary
>>>>>> > >>>>> to have such a bot but it would at least remind us to make a
>>>>>> decision
>>>>>> > >>>> about
>>>>>> > >>>>> older PRs with no activity.
>>>>>> > >>>>>
>>>>>> > >>>>> Cheers,
>>>>>> > >>>>> Till
>>>>>> > >>>>>
>>>>>> > >>>>> On Tue, May 15, 2018 at 3:26 PM, Chesnay Schepler <
>>>>>> chesnay@apache.org>
>>>>>> > >>>>> wrote:
>>>>>> > >>>>>
>>>>>> > >>>>>> /So far I did it twice for older PRs. In both cases I didn’t
>>>>>> get
>>>>>> any
>>>>>> > >>>>>> response and I even forgot in which PRs I had asked this
>>>>>> question,
>>>>>> so
>>>>>> > >>>>> now I
>>>>>> > >>>>>> can not even close them :S/
>>>>>> > >>>>>>
>>>>>> > >>>>>> To be honest this sounds more like an issue with how your
>>>>>> organize
>>>>>> > >> your
>>>>>> > >>>>>> work. No amount of closing PRs can fix that.
>>>>>> > >>>>>> With GitBox we can assign reviewers to a PR, but I'm not sure
>>>>>> whether
>>>>>> > >>>> it
>>>>>> > >>>>>> only allows committers to assign people.
>>>>>> > >>>>>> Bookmarks or text files might help as well./
>>>>>> > >>>>>> /
>>>>>> > >>>>>>
>>>>>> > >>>>>> /Regarding only 30% blocked on contributor. I wonder what
>>>>>> would be
>>>>>> > >> this
>>>>>> > >>>>>> number if we tried to ask in the rest of old PRs “Hey, are
>>>>>> you
>>>>>> still
>>>>>> > >>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>> waiting for a
>>>>>> > >>>>>> reviewer for X months, it doesn’t mean that’s not abandoned.
>>>>>> Even
>>>>>> if
>>>>>> > >> it
>>>>>> > >>>>>> doesn’t, reducing our overhead by those 30% of the PRs is
>>>>>> something./
>>>>>> > >>>>>>
>>>>>> > >>>>>> No doubt the number would be higher if we were to go back,
>>>>>> but as i
>>>>>> > >>>>>> explained earlier that is not a reason to close it. If a PR
>>>>>> is
>>>>>> > >>>> abandoned
>>>>>> > >>>>>> because we messed up we should still /try /to get it in.
>>>>>> > >>>>>>
>>>>>> > >>>>>> /This is kind of whole point of what I was proposing. If the
>>>>>> PR
>>>>>> author
>>>>>> > >>>> is
>>>>>> > >>>>>> still there, and can respond/bump/interrupt the closing
>>>>>> timeout,
>>>>>> > >> that’s
>>>>>> > >>>>>> great. Gives us even more sense of urgency to review it./
>>>>>> > >>>>>>
>>>>>> > >>>>>> Unfortunately knowing that it is more urgent is irrelevant,
>>>>>> as we
>>>>>> > >>>>>> currently don't have the manpower to review them. Reviving
>>>>>> them now
>>>>>> > >>>> would
>>>>>> > >>>>>> serve no purpose. The alternative is to wait until more
>>>>>> people
>>>>>> become
>>>>>> > >>>>>> active reviewers.
>>>>>> > >>>>>>
>>>>>> > >>>>>> /As a last resort, closing PR after timeout is not the end
>>>>>> of the
>>>>>> > >>>> world.
>>>>>> > >>>>>> It always can be reopened./
>>>>>> > >>>>>>
>>>>>> > >>>>>> Let's be realistic here, it will not be reopened.
>>>>>> > >>>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>> On 15.05.2018 14:21, Piotr Nowojski wrote:
>>>>>> > >>>>>>
>>>>>> > >>>>>>> I agree that we have other, even more important, problems
>>>>>> with
>>>>>> > >>>> reviewing
>>>>>> > >>>>>>> PR and community, but that shouldn’t block us from trying to
>>>>>> clean up
>>>>>> > >>>>>>> things a little bit and minimise the effort needed for
>>>>>> reviewing
>>>>>> PRs.
>>>>>> > >>>>> Now
>>>>>> > >>>>>>> before reviewing/picking older PRs I had to ask this “Hey,
>>>>>> are you
>>>>>> > >>>> still
>>>>>> > >>>>>>> interested in merging this?” manually and wait for the
>>>>>> response.
>>>>>> If
>>>>>> > >> it
>>>>>> > >>>>>>> doesn’t come, I have to remember to go back and close PR,
>>>>>> which I
>>>>>> of
>>>>>> > >>>>> course
>>>>>> > >>>>>>> forget to do. Bah, so far I did it twice for older PRs. In
>>>>>> both
>>>>>> cases
>>>>>> > >>>> I
>>>>>> > >>>>>>> didn’t get any response and I even forgot in which PRs I
>>>>>> had asked
>>>>>> > >>>> this
>>>>>> > >>>>>>> question, so now I can not even close them :S Wasted effort
>>>>>> and
>>>>>> > >> wasted
>>>>>> > >>>>> time
>>>>>> > >>>>>>> on context switching for me and for everyone else that will
>>>>>> have
>>>>>> to
>>>>>> > >>>>> scroll
>>>>>> > >>>>>>> pass or look on those PR to check their status.
>>>>>> > >>>>>>>
>>>>>> > >>>>>>> Keep in mind that I am not proposing to close the PR
>>>>>> automatically
>>>>>> > >>>>>>> straight on after 3 months of inactivity. Only after asking
>>>>>> a
>>>>>> > >> question
>>>>>> > >>>>>>> whether original contributor is still there and he is
>>>>>> interested
>>>>>> in
>>>>>> > >>>> the
>>>>>> > >>>>> PR
>>>>>> > >>>>>>> to be reviewed.
>>>>>> > >>>>>>>
>>>>>> > >>>>>>> for Flink 1.5, I merged a contribution from PR #1990 after
>>>>>> it was
>>>>>> > >>>>>>>> requested a few times by users.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>> This is kind of whole point of what I was proposing. If the
>>>>>> PR
>>>>>> author
>>>>>> > >>>> is
>>>>>> > >>>>>>> still there, and can respond/bump/interrupt the closing
>>>>>> timeout,
>>>>>> > >>>> that’s
>>>>>> > >>>>>>> great. Gives us even more sense of urgency to review it. On
>>>>>> the
>>>>>> other
>>>>>> > >>>>> hand
>>>>>> > >>>>>>> if there is no response (no interest from the author for
>>>>>> whatever
>>>>>> the
>>>>>> > >>>>>>> reasons) and nobody so far has picked this PR to
>>>>>> review/merge,
>>>>>> what’s
>>>>>> > >>>>> the
>>>>>> > >>>>>>> point of keeping such PR open? As a last resort, closing PR
>>>>>> after
>>>>>> > >>>>> timeout
>>>>>> > >>>>>>> is not the end of the world. It always can be reopened.
>>>>>> > >>>>>>>
>>>>>> > >>>>>>> Regarding only 30% blocked on contributor. I wonder what
>>>>>> would be
>>>>>> > >> this
>>>>>> > >>>>>>> number if we tried to ask in the rest of old PRs “Hey, are
>>>>>> you
>>>>>> still
>>>>>> > >>>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>> waiting for a
>>>>>> > >>>>> reviewer
>>>>>> > >>>>>>> for X months, it doesn’t mean that’s not abandoned. Even if
>>>>>> it
>>>>>> > >>>> doesn’t,
>>>>>> > >>>>>>> reducing our overhead by those 30% of the PRs is something.
>>>>>> > >>>>>>>
>>>>>> > >>>>>>> Piotrek
>>>>>> > >>>>>>>
>>>>>> > >>>>>>> On 15 May 2018, at 10:10, Fabian Hueske <fh...@gmail.com>
>>>>>> wrote:
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> I'm with Chesnay on this issue.
>>>>>> > >>>>>>>> Stale PRs, i.e., a PR where a contributor becomes
>>>>>> inactive, are
>>>>>> one
>>>>>> > >>>> of
>>>>>> > >>>>>>>> our
>>>>>> > >>>>>>>> smallest issues, IMO.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> There are more reasons for the high number of PRs.
>>>>>> > >>>>>>>> * Lack of timely reviews.
>>>>>> > >>>>>>>> * Not eagerly closing PRs that have no or very little
>>>>>> chance of
>>>>>> > >> being
>>>>>> > >>>>>>>> merged. Common reasons are
>>>>>> > >>>>>>>> 1) lack of interest in the feature by committers,
>>>>>> > >>>>>>>> 2) too extensive changes and hence time consuming reviews,
>>>>>> or
>>>>>> > >>>>>>>> 3) bad quality.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> For 1), there are lots of older JIRA issues, that have low
>>>>>> priority
>>>>>> > >>>> but
>>>>>> > >>>>>>>> which are picked up by contributors. In the contribution
>>>>>> guidelines
>>>>>> > >>>> we
>>>>>> > >>>>>>>> ask
>>>>>> > >>>>>>>> contributors to let us know when they want to work on an
>>>>>> issue.
>>>>>> So
>>>>>> > >>>> far
>>>>>> > >>>>>>>> our
>>>>>> > >>>>>>>> attitude has been, yes sure go ahead. I've seen very little
>>>>>> attempts
>>>>>> > >>>> of
>>>>>> > >>>>>>>> warning somebody to work on issues that won't be easily
>>>>>> merged.
>>>>>> > >>>>>>>> Once a PR has been opened, we should also be honest and let
>>>>>> > >>>>> contributors
>>>>>> > >>>>>>>> know that it has no chance or might take a while to get
>>>>>> reviewed.
>>>>>> > >>>>>>>> For 2) this is typically not so much of an issue if the
>>>>>> feature
>>>>>> is
>>>>>> > >>>>>>>> interesting. However, if 1) and 2) meet, chances to get a
>>>>>> change
>>>>>> in
>>>>>> > >>>>> drop
>>>>>> > >>>>>>>> even more.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> A common "strategy" to deal with PRs that fall into 1),
>>>>>> 2), or
>>>>>> 3) is
>>>>>> > >>>> to
>>>>>> > >>>>>>>> not
>>>>>> > >>>>>>>> look at them or giving a shallow review.
>>>>>> > >>>>>>>> Of course, contributors become unresponsive if we don't
>>>>>> look at
>>>>>> > >> their
>>>>>> > >>>>> PRs
>>>>>> > >>>>>>>> for weeks or months. But that's not their fault.
>>>>>> > >>>>>>>> Instead, I think we should be honest and communicate the
>>>>>> chances
>>>>>> of
>>>>>> > >> a
>>>>>> > >>>>> PR
>>>>>> > >>>>>>>> more clearly.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> Browsing over the list of open PRs, I feel that most older
>>>>>> PRs
>>>>>> fall
>>>>>> > >>>>> into
>>>>>> > >>>>>>>> the category of low-priority (or even unwanted) features.
>>>>>> > >>>>>>>> Bug fixes or features that the committers care about
>>>>>> usually
>>>>>> make it
>>>>>> > >>>>> into
>>>>>> > >>>>>>>> the code base.
>>>>>> > >>>>>>>> In case a contributor becomes inactive, committers often
>>>>>> take
>>>>>> over
>>>>>> > >> an
>>>>>> > >>>>>>>> push
>>>>>> > >>>>>>>> a contribution over the line.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> So, I do not support an auto-close mechanism.
>>>>>> > >>>>>>>> I think a better way to address the issue is better
>>>>>> communication,
>>>>>> > >>>> more
>>>>>> > >>>>>>>> eagerly closing PRs with no chance, and putting more
>>>>>> reviewing
>>>>>> > >>>> effort.
>>>>>> > >>>>>>>> IMO, we should only eagerly close PRs that have no chance
>>>>>> of
>>>>>> being
>>>>>> > >>>>>>>> merged.
>>>>>> > >>>>>>>> PRs with low-prio features might be picked up later (for
>>>>>> Flink
>>>>>> 1.5,
>>>>>> > >> I
>>>>>> > >>>>>>>> merged a contribution from PR #1990 after it was requested
>>>>>> a few
>>>>>> > >>>> times
>>>>>> > >>>>> by
>>>>>> > >>>>>>>> users).
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> However, I think we could do a pass over the oldest PRs and
>>>>>> check if
>>>>>> > >>>> we
>>>>>> > >>>>>>>> can
>>>>>> > >>>>>>>> close a bunch.
>>>>>> > >>>>>>>> There are quite a few contributions (many for flink-ml)
>>>>>> that I
>>>>>> don't
>>>>>> > >>>>> see
>>>>>> > >>>>>>>> a
>>>>>> > >>>>>>>> chance for getting merged.
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> Best, Fabian
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> -
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> 2018-05-15 9:13 GMT+02:00 Chesnay Schepler <
>>>>>> chesnay@apache.org>:
>>>>>> > >>>>>>>>
>>>>>> > >>>>>>>> -1
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> For clarification (since the original mail indicates
>>>>>> otherwise),
>>>>>> > >> the
>>>>>> > >>>>>>>>> number of pull requests that this would affect is fairly
>>>>>> small.
>>>>>> > >>>>>>>>> Only about 25-30% of all open PRs are blocked on the
>>>>>> contributor,
>>>>>> > >>>> the
>>>>>> > >>>>>>>>> remaining ones are actually blocked on the review.
>>>>>> > >>>>>>>>> Thus is reject the premise that one has to search through
>>>>>> that
>>>>>> many
>>>>>> > >>>>> PRs
>>>>>> > >>>>>>>>> to
>>>>>> > >>>>>>>>> find something to review, there is plenty.
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> I believe it to be highly unfair for us to close PRs due
>>>>>> to
>>>>>> > >>>>> inactivity,
>>>>>> > >>>>>>>>> when the primary cause has been /our /own inactivity.
>>>>>> > >>>>>>>>> If a PR is opened and the first comment comes in 3 months
>>>>>> late,
>>>>>> > >>>> then I
>>>>>> > >>>>>>>>> don't blame the contributor for not responding.
>>>>>> > >>>>>>>>> IMO we owe it to the contributor to evaluate their PR,
>>>>>> and if
>>>>>> > >>>>> necessary
>>>>>> > >>>>>>>>> bring it to a merge-able state (to a certain extend).
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> There's also the fact that closing these PRs outright
>>>>>> would
>>>>>> waste a
>>>>>> > >>>>> lot
>>>>>> > >>>>>>>>> of
>>>>>> > >>>>>>>>> good contributions.
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> Finally, this solution is overkill for what we want to
>>>>>> achieve.
>>>>>> If
>>>>>> > >>>> we
>>>>>> > >>>>>>>>> want
>>>>>> > >>>>>>>>> to make it easier to find PRs to review all we need is
>>>>>> > >>>>>>>>> GitBox integration and tagging or PRs. That's it. We
>>>>>> could have
>>>>>> a
>>>>>> > >>>>> /fully
>>>>>> > >>>>>>>>> /tagged PR list /tomorrow/, if we really wanted to.
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> On 15.05.2018 05:10, Ted Yu wrote:
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>>> bq. this pull request requires a review, please simply
>>>>>> write any
>>>>>> > >>>>>>>>>> comment.
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>> Shouldn't the wording of such comment be known before
>>>>>> hand ?
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>> Otherwise pull request waiting for committers' review
>>>>>> may be
>>>>>> > >>>>>>>>>> mis-classified.
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>> Cheers
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>> On Mon, May 14, 2018 at 7:59 PM, blues zheng <
>>>>>> kisimple@163.com>
>>>>>> > >>>>> wrote:
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>> +1 for the proposal.
>>>>>> > >>>>>>>>>>
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> Best,
>>>>>> > >>>>>>>>>>> blues
>>>>>> > >>>>>>>>>>> On 05/14/2018 20:58, Ufuk Celebi wrote:
>>>>>> > >>>>>>>>>>> Hey Piotr,
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> thanks for bringing this up. I really like this
>>>>>> proposal and
>>>>>> also
>>>>>> > >>>>> saw
>>>>>> > >>>>>>>>>>> it work successfully at other projects. So +1 from my
>>>>>> side.
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> - I like the approach with a notification one week
>>>>>> before
>>>>>> > >>>>>>>>>>> automatically closing the PR
>>>>>> > >>>>>>>>>>> - I think a bot will the best option as these kinds of
>>>>>> things
>>>>>> are
>>>>>> > >>>>>>>>>>> usually followed enthusiastically in the beginning but
>>>>>> eventually
>>>>>> > >>>>>>>>>>> loose traction
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> We can enable better integration with GitHub by using
>>>>>> ASF
>>>>>> GitBox
>>>>>> > >>>>>>>>>>> (https://gitbox.apache.org/setup/) but we should
>>>>>> discuss that
>>>>>> in
>>>>>> > >>>> a
>>>>>> > >>>>>>>>>>> separate thread.
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> – Ufuk
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> On Mon, May 14, 2018 at 12:04 PM, Piotr Nowojski
>>>>>> > >>>>>>>>>>> <pi...@data-artisans.com> wrote:
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> Hey,
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> We have lots of open pull requests and quite some of
>>>>>> them are
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> stale/abandoned/inactive. Often such old PRs are
>>>>>> impossible
>>>>>> to
>>>>>> > >>>>> merge
>>>>>> > >>>>>>>>>>> due
>>>>>> > >>>>>>>>>>> to
>>>>>> > >>>>>>>>>>> conflicts and it’s easier to just abandon and rewrite
>>>>>> them.
>>>>>> > >>>>> Especially
>>>>>> > >>>>>>>>>>> there are some PRs which original contributor created
>>>>>> long
>>>>>> time
>>>>>> > >>>> ago,
>>>>>> > >>>>>>>>>>> someone else wrote some comments/review and… that’s
>>>>>> about it.
>>>>>> > >>>>> Original
>>>>>> > >>>>>>>>>>> contributor never shown up again to respond to the
>>>>>> comments.
>>>>>> > >>>>>>>>>>> Regardless
>>>>>> > >>>>>>>>>>> of
>>>>>> > >>>>>>>>>>> the reason such PRs are clogging the GitHub, making it
>>>>>> difficult
>>>>>> > >>>> to
>>>>>> > >>>>>>>>>>> keep
>>>>>> > >>>>>>>>>>> track of things and making it almost impossible to find
>>>>>> a
>>>>>> little
>>>>>> > >>>> bit
>>>>>> > >>>>>>>>>>> old
>>>>>> > >>>>>>>>>>> (for example 3+ months) PRs that are still valid and
>>>>>> waiting
>>>>>> for
>>>>>> > >>>>>>>>>>> reviews.
>>>>>> > >>>>>>>>>>> To do something like that, one would have to dig
>>>>>> through tens
>>>>>> or
>>>>>> > >>>>>>>>>>> hundreds
>>>>>> > >>>>>>>>>>> of abandoned PRs.
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> What I would like to propose is to agree on some
>>>>>> inactivity
>>>>>> dead
>>>>>> > >>>>> line,
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> lets say 3 months. After crossing such deadline, PRs
>>>>>> should
>>>>>> be
>>>>>> > >>>>>>>>>>> marked/commented as “stale”, with information like:
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> “This pull request has been marked as stale due to 3
>>>>>> months of
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> inactivity. It will be closed in 1 week if no further
>>>>>> activity
>>>>>> > >>>>>>>>>>> occurs. If
>>>>>> > >>>>>>>>>>> you think that’s incorrect or this pull request
>>>>>> requires a
>>>>>> > >> review,
>>>>>> > >>>>>>>>>>> please
>>>>>> > >>>>>>>>>>> simply write any comment.”
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> Either we could just agree on such policy and enforce it
>>>>>> manually
>>>>>> > >>>>>>>>>>>> (maybe
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> with some simple tooling, like a simple script to list
>>>>>> inactive
>>>>>> > >>>>> PRs -
>>>>>> > >>>>>>>>>>> seems
>>>>>> > >>>>>>>>>>> like couple of lines in python by using PyGithub) or we
>>>>>> could
>>>>>> > >>>> think
>>>>>> > >>>>>>>>>>> about
>>>>>> > >>>>>>>>>>> automating this action. There are some bots that do
>>>>>> exactly
>>>>>> this
>>>>>> > >>>>> (like
>>>>>> > >>>>>>>>>>> this
>>>>>> > >>>>>>>>>>> one: https://github.com/probot/stale <
>>>>>> https://github.com/probot/
>>>>>> > >>>>> stale
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>> ),
>>>>>> > >>>>>>>>>>> but probably they would need to be adopted to
>>>>>> limitations of
>>>>>> our
>>>>>> > >>>>>>>>>>> Apache
>>>>>> > >>>>>>>>>>> repository (we can not add labels and we can not close
>>>>>> the PRs
>>>>>> > >> via
>>>>>> > >>>>>>>>>>> GitHub).
>>>>>> > >>>>>>>>>>>
>>>>>> > >>>>>>>>>>> What do you think about it?
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>> Piotrek
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>>>>
>>>>>> > >>>>>>>>>
>>>>>> > >>>>>>>
>>>>>> > >>>>>>
>>>>>> > >>>>>
>>>>>> > >>>>
>>>>>> > >>
>>>>>> > >>
>>>>>>
>>>>>
Re: Closing (automatically?) inactive pull requests
Posted by Alan Myrvold <am...@google.com>.
A vote makes sense.
The proposed config is at https://github.com/apache/beam/pull/5532/files
and will mark pull requests as stale after 90 days and close them 7 days
later.
Issues are not affected.
On Fri, Jun 1, 2018 at 9:14 AM Kenneth Knowles <kl...@google.com> wrote:
> Ismael had mentioned to vote on this, so let's do that now to make sure no
> one missed this discussion.
>
> On Thu, May 31, 2018 at 9:33 PM Alan Myrvold <am...@google.com> wrote:
>
>> Thanks. I can look into adding the stale.yaml file for old pull requests/
>>
>> On Thu, May 31, 2018 at 8:07 PM Kenneth Knowles <kl...@google.com> wrote:
>>
>>> Update: you brought the information needed, and it is now enabled.
>>> Thanks for the follow-through!
>>>
>>> Since you dug into probot's details, I took the liberty of assigning
>>> BEAM-4423 to you, in case throwing together the needed configs is fresh in
>>> your mind and you are in the mood to continue. (if not, pass the hot
>>> potato, back to me is OK too :-)
>>>
>>> Kenn
>>>
>>> On Thu, May 31, 2018 at 4:50 PM Alan Myrvold <am...@google.com>
>>> wrote:
>>>
>>>> INFRA-16589 got closed asking to clarify that the probot-stale app
>>>> would not have permissions to merge automatically.
>>>> From my reading of the permissions documentation, it would not. I added
>>>> a comment to INFRA-16589
>>>>
>>>> On Tue, May 29, 2018 at 10:05 AM Lukasz Cwik <lc...@google.com> wrote:
>>>>
>>>>> I opened up INFRA-16589
>>>>> <https://issues.apache.org/jira/browse/INFRA-16589> for Apache infra
>>>>> to install Stale but they denied a similar request INFRA-16394
>>>>> <https://issues.apache.org/jira/browse/INFRA-16394> because of
>>>>> permissions issues. I tried clarifying the permissions requested.
>>>>>
>>>>> On Tue, May 29, 2018 at 9:39 AM Scott Wegner <sw...@google.com>
>>>>> wrote:
>>>>>
>>>>>> +1. I've opened BEAM-4423 to capture the discussion here:
>>>>>> https://issues.apache.org/jira/browse/BEAM-4423
>>>>>>
>>>>>> On Thu, May 24, 2018 at 5:34 PM Chamikara Jayalath <
>>>>>> chamikara@google.com> wrote:
>>>>>>
>>>>>>> +1 for automatically closing. Currently, contribution guide mentions
>>>>>>> that pull requests without responses to actionable comments become stale
>>>>>>> (and closed) after two months but last I checked there were many pull
>>>>>>> requests that met this criteria but had not been closed after many months.
>>>>>>> So seems like humans are reluctant to act on the established policy :)
>>>>>>>
>>>>>>> - Cham
>>>>>>>
>>>>>>> On Wed, May 23, 2018 at 11:25 AM Kenneth Knowles <kl...@google.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> That makes sense, to just focus on Beam's decision. It seems the
>>>>>>>> tool is already built. I thought we just had to deploy it, but maybe not
>>>>>>>> even that, if we can just activate it:
>>>>>>>> https://github.com/apps/stale
>>>>>>>>
>>>>>>>> Kenn
>>>>>>>>
>>>>>>>> On Wed, May 23, 2018 at 9:31 AM Ismaël Mejía <ie...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Given that reaching consensus in both communities seems like a
>>>>>>>>> harder task
>>>>>>>>> than just deciding our policy. in the Beam side Why don't we just
>>>>>>>>> go ahead
>>>>>>>>> and vote around this + build the tool, and if the Flink guys are
>>>>>>>>> interested
>>>>>>>>> they can take it, no?
>>>>>>>>>
>>>>>>>>> in the future we can share that code.
>>>>>>>>> On Wed, May 16, 2018 at 3:31 PM Piotr Nowojski <
>>>>>>>>> piotr@data-artisans.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> > The question is what would such tool offer on top of over a
>>>>>>>>> Github’s view
>>>>>>>>> of PR sorted by “least recently updated”:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> https://github.com/apache/flink/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc
>>>>>>>>> <
>>>>>>>>> https://github.com/apache/flink/pulls?q=is:pr+is:open+sort:updated-asc
>>>>>>>>> >
>>>>>>>>>
>>>>>>>>> > ? Maybe it would be good enough to have a policy about waiting x
>>>>>>>>> months/weeks for a contributor to respond. If he doesn’t, we
>>>>>>>>> either take
>>>>>>>>> over PR we or close it. Having “clean” view of oldest PRs would be
>>>>>>>>> beneficial for reviewing PRs in ~FIFO order as part of community
>>>>>>>>> work.
>>>>>>>>>
>>>>>>>>> > Having
>>>>>>>>>
>>>>>>>>> > > On 16 May 2018, at 10:57, Fabian Hueske <fh...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> > >
>>>>>>>>> > > Hi,
>>>>>>>>> > >
>>>>>>>>> > > I'm not objecting closing stale PRs.
>>>>>>>>> > > We have quite a few PRs with very little chance of being
>>>>>>>>> merged and I
>>>>>>>>> would
>>>>>>>>> > > certainly appreciate cleaning up those.
>>>>>>>>> > > However, I think we should not automate closing PRs for the
>>>>>>>>> reasons I
>>>>>>>>> gave
>>>>>>>>> > > before.
>>>>>>>>> > >
>>>>>>>>> > > A tool that reminds us of state PRs as proposed by Till seems
>>>>>>>>> like a
>>>>>>>>> good
>>>>>>>>> > > idea and might actually help to re-adjust priorities from time
>>>>>>>>> to time.
>>>>>>>>> > >
>>>>>>>>> > > @Yazdan: Right now there is no assignment happening.
>>>>>>>>> Committers decide
>>>>>>>>> > > which PRs they want to review and merge.
>>>>>>>>> > >
>>>>>>>>> > > Best, Fabian
>>>>>>>>> > >
>>>>>>>>> > > 2018-05-16 4:26 GMT+02:00 Yaz Sh <ya...@gmail.com>:
>>>>>>>>> > >
>>>>>>>>> > >> I have questions in this regard (you guys might have
>>>>>>>>> addressed it in
>>>>>>>>> this
>>>>>>>>> > >> email chain):
>>>>>>>>> > >> how PRs get assigned to a reviewer apart of a contributor tag
>>>>>>>>> someone?
>>>>>>>>> > >> what if PR never gets a reviewer attention and it became
>>>>>>>>> in-active due
>>>>>>>>> to
>>>>>>>>> > >> long review respond? should Bot assign a reviewer to a PR
>>>>>>>>> based on
>>>>>>>>> > >> reviewers interest (i.e., defined via tags) and notify he/she
>>>>>>>>> if PR is
>>>>>>>>> > >> waiting for review?
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>> > >> Cheers,
>>>>>>>>> > >> /Yazdan
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>> > >>> On May 15, 2018, at 12:06 PM, Thomas Weise <th...@apache.org>
>>>>>>>>> wrote:
>>>>>>>>> > >>>
>>>>>>>>> > >>> I like Till's proposal to notify the participants on the PR
>>>>>>>>> to PTAL.
>>>>>>>>> But
>>>>>>>>> > >> I
>>>>>>>>> > >>> would also suggest to auto-close when no action is taken,
>>>>>>>>> with a
>>>>>>>>> friendly
>>>>>>>>> > >>> reminder that PRs can be reopened anytime.
>>>>>>>>> > >>>
>>>>>>>>> > >>> The current situation with 350 open PRs may send a signal to
>>>>>>>>> contributors
>>>>>>>>> > >>> that it may actually be too much hassle to get a change
>>>>>>>>> committed in
>>>>>>>>> > >> Flink.
>>>>>>>>> > >>> Since the count keeps on rising, this is also not a past
>>>>>>>>> problem.
>>>>>>>>> Pruning
>>>>>>>>> > >>> inactive PRs may help, that will also give a more accurate
>>>>>>>>> picture of
>>>>>>>>> the
>>>>>>>>> > >>> lack of review bandwidth, if that is the root cause.
>>>>>>>>> > >>>
>>>>>>>>> > >>> Thanks,
>>>>>>>>> > >>> Thomas
>>>>>>>>> > >>>
>>>>>>>>> > >>>
>>>>>>>>> > >>>
>>>>>>>>> > >>>
>>>>>>>>> > >>>
>>>>>>>>> > >>> On Tue, May 15, 2018 at 8:24 AM, Ted Yu <yu...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> > >>>
>>>>>>>>> > >>>> How does the bot decide whether the PR is waiting for
>>>>>>>>> reviews or is
>>>>>>>>> > >> being
>>>>>>>>> > >>>> abandoned by contributor ?
>>>>>>>>> > >>>>
>>>>>>>>> > >>>> How about letting the bot count the number of times
>>>>>>>>> contributor pings
>>>>>>>>> > >>>> committer(s) for review ?
>>>>>>>>> > >>>> When unanswered ping count crosses some threshold, say 3,
>>>>>>>>> the bot
>>>>>>>>> > >> publishes
>>>>>>>>> > >>>> the JIRA and PR somewhere.
>>>>>>>>> > >>>>
>>>>>>>>> > >>>> Cheers
>>>>>>>>> > >>>>
>>>>>>>>> > >>>> On Tue, May 15, 2018 at 8:19 AM, Till Rohrmann <
>>>>>>>>> trohrmann@apache.org>
>>>>>>>>> > >>>> wrote:
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>> I'm a bit torn here because I can see the pros and cons
>>>>>>>>> for both
>>>>>>>>> sides.
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> Maybe a compromise could be to not have a closing but a
>>>>>>>>> monitoring
>>>>>>>>> bot
>>>>>>>>> > >>>>> which notifies us about inactive PRs. This could then
>>>>>>>>> trigger an
>>>>>>>>> > >>>>> investigation of the underlying problem and ultimately
>>>>>>>>> lead to a
>>>>>>>>> > >>>> conscious
>>>>>>>>> > >>>>> decision to close or keep the PR open. As such it is not
>>>>>>>>> strictly
>>>>>>>>> > >>>> necessary
>>>>>>>>> > >>>>> to have such a bot but it would at least remind us to make
>>>>>>>>> a
>>>>>>>>> decision
>>>>>>>>> > >>>> about
>>>>>>>>> > >>>>> older PRs with no activity.
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> Cheers,
>>>>>>>>> > >>>>> Till
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>> On Tue, May 15, 2018 at 3:26 PM, Chesnay Schepler <
>>>>>>>>> chesnay@apache.org>
>>>>>>>>> > >>>>> wrote:
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>>> /So far I did it twice for older PRs. In both cases I
>>>>>>>>> didn’t get
>>>>>>>>> any
>>>>>>>>> > >>>>>> response and I even forgot in which PRs I had asked this
>>>>>>>>> question,
>>>>>>>>> so
>>>>>>>>> > >>>>> now I
>>>>>>>>> > >>>>>> can not even close them :S/
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> To be honest this sounds more like an issue with how your
>>>>>>>>> organize
>>>>>>>>> > >> your
>>>>>>>>> > >>>>>> work. No amount of closing PRs can fix that.
>>>>>>>>> > >>>>>> With GitBox we can assign reviewers to a PR, but I'm not
>>>>>>>>> sure
>>>>>>>>> whether
>>>>>>>>> > >>>> it
>>>>>>>>> > >>>>>> only allows committers to assign people.
>>>>>>>>> > >>>>>> Bookmarks or text files might help as well./
>>>>>>>>> > >>>>>> /
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> /Regarding only 30% blocked on contributor. I wonder what
>>>>>>>>> would be
>>>>>>>>> > >> this
>>>>>>>>> > >>>>>> number if we tried to ask in the rest of old PRs “Hey,
>>>>>>>>> are you
>>>>>>>>> still
>>>>>>>>> > >>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>>>> waiting for a
>>>>>>>>> > >>>>>> reviewer for X months, it doesn’t mean that’s not
>>>>>>>>> abandoned. Even
>>>>>>>>> if
>>>>>>>>> > >> it
>>>>>>>>> > >>>>>> doesn’t, reducing our overhead by those 30% of the PRs is
>>>>>>>>> something./
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> No doubt the number would be higher if we were to go
>>>>>>>>> back, but as i
>>>>>>>>> > >>>>>> explained earlier that is not a reason to close it. If a
>>>>>>>>> PR is
>>>>>>>>> > >>>> abandoned
>>>>>>>>> > >>>>>> because we messed up we should still /try /to get it in.
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> /This is kind of whole point of what I was proposing. If
>>>>>>>>> the PR
>>>>>>>>> author
>>>>>>>>> > >>>> is
>>>>>>>>> > >>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>>>> timeout,
>>>>>>>>> > >> that’s
>>>>>>>>> > >>>>>> great. Gives us even more sense of urgency to review it./
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> Unfortunately knowing that it is more urgent is
>>>>>>>>> irrelevant, as we
>>>>>>>>> > >>>>>> currently don't have the manpower to review them.
>>>>>>>>> Reviving them now
>>>>>>>>> > >>>> would
>>>>>>>>> > >>>>>> serve no purpose. The alternative is to wait until more
>>>>>>>>> people
>>>>>>>>> become
>>>>>>>>> > >>>>>> active reviewers.
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> /As a last resort, closing PR after timeout is not the
>>>>>>>>> end of the
>>>>>>>>> > >>>> world.
>>>>>>>>> > >>>>>> It always can be reopened./
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> Let's be realistic here, it will not be reopened.
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>> On 15.05.2018 14:21, Piotr Nowojski wrote:
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>>> I agree that we have other, even more important,
>>>>>>>>> problems with
>>>>>>>>> > >>>> reviewing
>>>>>>>>> > >>>>>>> PR and community, but that shouldn’t block us from
>>>>>>>>> trying to
>>>>>>>>> clean up
>>>>>>>>> > >>>>>>> things a little bit and minimise the effort needed for
>>>>>>>>> reviewing
>>>>>>>>> PRs.
>>>>>>>>> > >>>>> Now
>>>>>>>>> > >>>>>>> before reviewing/picking older PRs I had to ask this
>>>>>>>>> “Hey, are you
>>>>>>>>> > >>>> still
>>>>>>>>> > >>>>>>> interested in merging this?” manually and wait for the
>>>>>>>>> response.
>>>>>>>>> If
>>>>>>>>> > >> it
>>>>>>>>> > >>>>>>> doesn’t come, I have to remember to go back and close
>>>>>>>>> PR, which I
>>>>>>>>> of
>>>>>>>>> > >>>>> course
>>>>>>>>> > >>>>>>> forget to do. Bah, so far I did it twice for older PRs.
>>>>>>>>> In both
>>>>>>>>> cases
>>>>>>>>> > >>>> I
>>>>>>>>> > >>>>>>> didn’t get any response and I even forgot in which PRs I
>>>>>>>>> had asked
>>>>>>>>> > >>>> this
>>>>>>>>> > >>>>>>> question, so now I can not even close them :S Wasted
>>>>>>>>> effort and
>>>>>>>>> > >> wasted
>>>>>>>>> > >>>>> time
>>>>>>>>> > >>>>>>> on context switching for me and for everyone else that
>>>>>>>>> will have
>>>>>>>>> to
>>>>>>>>> > >>>>> scroll
>>>>>>>>> > >>>>>>> pass or look on those PR to check their status.
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>> Keep in mind that I am not proposing to close the PR
>>>>>>>>> automatically
>>>>>>>>> > >>>>>>> straight on after 3 months of inactivity. Only after
>>>>>>>>> asking a
>>>>>>>>> > >> question
>>>>>>>>> > >>>>>>> whether original contributor is still there and he is
>>>>>>>>> interested
>>>>>>>>> in
>>>>>>>>> > >>>> the
>>>>>>>>> > >>>>> PR
>>>>>>>>> > >>>>>>> to be reviewed.
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>> for Flink 1.5, I merged a contribution from PR #1990
>>>>>>>>> after it was
>>>>>>>>> > >>>>>>>> requested a few times by users.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>> This is kind of whole point of what I was proposing. If
>>>>>>>>> the PR
>>>>>>>>> author
>>>>>>>>> > >>>> is
>>>>>>>>> > >>>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>>>> timeout,
>>>>>>>>> > >>>> that’s
>>>>>>>>> > >>>>>>> great. Gives us even more sense of urgency to review it.
>>>>>>>>> On the
>>>>>>>>> other
>>>>>>>>> > >>>>> hand
>>>>>>>>> > >>>>>>> if there is no response (no interest from the author for
>>>>>>>>> whatever
>>>>>>>>> the
>>>>>>>>> > >>>>>>> reasons) and nobody so far has picked this PR to
>>>>>>>>> review/merge,
>>>>>>>>> what’s
>>>>>>>>> > >>>>> the
>>>>>>>>> > >>>>>>> point of keeping such PR open? As a last resort, closing
>>>>>>>>> PR after
>>>>>>>>> > >>>>> timeout
>>>>>>>>> > >>>>>>> is not the end of the world. It always can be reopened.
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>> Regarding only 30% blocked on contributor. I wonder what
>>>>>>>>> would be
>>>>>>>>> > >> this
>>>>>>>>> > >>>>>>> number if we tried to ask in the rest of old PRs “Hey,
>>>>>>>>> are you
>>>>>>>>> still
>>>>>>>>> > >>>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>>>> waiting for a
>>>>>>>>> > >>>>> reviewer
>>>>>>>>> > >>>>>>> for X months, it doesn’t mean that’s not abandoned. Even
>>>>>>>>> if it
>>>>>>>>> > >>>> doesn’t,
>>>>>>>>> > >>>>>>> reducing our overhead by those 30% of the PRs is
>>>>>>>>> something.
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>> Piotrek
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>> On 15 May 2018, at 10:10, Fabian Hueske <
>>>>>>>>> fhueske@gmail.com> wrote:
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> I'm with Chesnay on this issue.
>>>>>>>>> > >>>>>>>> Stale PRs, i.e., a PR where a contributor becomes
>>>>>>>>> inactive, are
>>>>>>>>> one
>>>>>>>>> > >>>> of
>>>>>>>>> > >>>>>>>> our
>>>>>>>>> > >>>>>>>> smallest issues, IMO.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> There are more reasons for the high number of PRs.
>>>>>>>>> > >>>>>>>> * Lack of timely reviews.
>>>>>>>>> > >>>>>>>> * Not eagerly closing PRs that have no or very little
>>>>>>>>> chance of
>>>>>>>>> > >> being
>>>>>>>>> > >>>>>>>> merged. Common reasons are
>>>>>>>>> > >>>>>>>> 1) lack of interest in the feature by committers,
>>>>>>>>> > >>>>>>>> 2) too extensive changes and hence time consuming
>>>>>>>>> reviews, or
>>>>>>>>> > >>>>>>>> 3) bad quality.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> For 1), there are lots of older JIRA issues, that have
>>>>>>>>> low
>>>>>>>>> priority
>>>>>>>>> > >>>> but
>>>>>>>>> > >>>>>>>> which are picked up by contributors. In the contribution
>>>>>>>>> guidelines
>>>>>>>>> > >>>> we
>>>>>>>>> > >>>>>>>> ask
>>>>>>>>> > >>>>>>>> contributors to let us know when they want to work on
>>>>>>>>> an issue.
>>>>>>>>> So
>>>>>>>>> > >>>> far
>>>>>>>>> > >>>>>>>> our
>>>>>>>>> > >>>>>>>> attitude has been, yes sure go ahead. I've seen very
>>>>>>>>> little
>>>>>>>>> attempts
>>>>>>>>> > >>>> of
>>>>>>>>> > >>>>>>>> warning somebody to work on issues that won't be easily
>>>>>>>>> merged.
>>>>>>>>> > >>>>>>>> Once a PR has been opened, we should also be honest and
>>>>>>>>> let
>>>>>>>>> > >>>>> contributors
>>>>>>>>> > >>>>>>>> know that it has no chance or might take a while to get
>>>>>>>>> reviewed.
>>>>>>>>> > >>>>>>>> For 2) this is typically not so much of an issue if the
>>>>>>>>> feature
>>>>>>>>> is
>>>>>>>>> > >>>>>>>> interesting. However, if 1) and 2) meet, chances to get
>>>>>>>>> a change
>>>>>>>>> in
>>>>>>>>> > >>>>> drop
>>>>>>>>> > >>>>>>>> even more.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> A common "strategy" to deal with PRs that fall into 1),
>>>>>>>>> 2), or
>>>>>>>>> 3) is
>>>>>>>>> > >>>> to
>>>>>>>>> > >>>>>>>> not
>>>>>>>>> > >>>>>>>> look at them or giving a shallow review.
>>>>>>>>> > >>>>>>>> Of course, contributors become unresponsive if we don't
>>>>>>>>> look at
>>>>>>>>> > >> their
>>>>>>>>> > >>>>> PRs
>>>>>>>>> > >>>>>>>> for weeks or months. But that's not their fault.
>>>>>>>>> > >>>>>>>> Instead, I think we should be honest and communicate
>>>>>>>>> the chances
>>>>>>>>> of
>>>>>>>>> > >> a
>>>>>>>>> > >>>>> PR
>>>>>>>>> > >>>>>>>> more clearly.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> Browsing over the list of open PRs, I feel that most
>>>>>>>>> older PRs
>>>>>>>>> fall
>>>>>>>>> > >>>>> into
>>>>>>>>> > >>>>>>>> the category of low-priority (or even unwanted)
>>>>>>>>> features.
>>>>>>>>> > >>>>>>>> Bug fixes or features that the committers care about
>>>>>>>>> usually
>>>>>>>>> make it
>>>>>>>>> > >>>>> into
>>>>>>>>> > >>>>>>>> the code base.
>>>>>>>>> > >>>>>>>> In case a contributor becomes inactive, committers
>>>>>>>>> often take
>>>>>>>>> over
>>>>>>>>> > >> an
>>>>>>>>> > >>>>>>>> push
>>>>>>>>> > >>>>>>>> a contribution over the line.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> So, I do not support an auto-close mechanism.
>>>>>>>>> > >>>>>>>> I think a better way to address the issue is better
>>>>>>>>> communication,
>>>>>>>>> > >>>> more
>>>>>>>>> > >>>>>>>> eagerly closing PRs with no chance, and putting more
>>>>>>>>> reviewing
>>>>>>>>> > >>>> effort.
>>>>>>>>> > >>>>>>>> IMO, we should only eagerly close PRs that have no
>>>>>>>>> chance of
>>>>>>>>> being
>>>>>>>>> > >>>>>>>> merged.
>>>>>>>>> > >>>>>>>> PRs with low-prio features might be picked up later
>>>>>>>>> (for Flink
>>>>>>>>> 1.5,
>>>>>>>>> > >> I
>>>>>>>>> > >>>>>>>> merged a contribution from PR #1990 after it was
>>>>>>>>> requested a few
>>>>>>>>> > >>>> times
>>>>>>>>> > >>>>> by
>>>>>>>>> > >>>>>>>> users).
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> However, I think we could do a pass over the oldest PRs
>>>>>>>>> and
>>>>>>>>> check if
>>>>>>>>> > >>>> we
>>>>>>>>> > >>>>>>>> can
>>>>>>>>> > >>>>>>>> close a bunch.
>>>>>>>>> > >>>>>>>> There are quite a few contributions (many for flink-ml)
>>>>>>>>> that I
>>>>>>>>> don't
>>>>>>>>> > >>>>> see
>>>>>>>>> > >>>>>>>> a
>>>>>>>>> > >>>>>>>> chance for getting merged.
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> Best, Fabian
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> -
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> 2018-05-15 9:13 GMT+02:00 Chesnay Schepler <
>>>>>>>>> chesnay@apache.org>:
>>>>>>>>> > >>>>>>>>
>>>>>>>>> > >>>>>>>> -1
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> For clarification (since the original mail indicates
>>>>>>>>> otherwise),
>>>>>>>>> > >> the
>>>>>>>>> > >>>>>>>>> number of pull requests that this would affect is
>>>>>>>>> fairly small.
>>>>>>>>> > >>>>>>>>> Only about 25-30% of all open PRs are blocked on the
>>>>>>>>> contributor,
>>>>>>>>> > >>>> the
>>>>>>>>> > >>>>>>>>> remaining ones are actually blocked on the review.
>>>>>>>>> > >>>>>>>>> Thus is reject the premise that one has to search
>>>>>>>>> through that
>>>>>>>>> many
>>>>>>>>> > >>>>> PRs
>>>>>>>>> > >>>>>>>>> to
>>>>>>>>> > >>>>>>>>> find something to review, there is plenty.
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> I believe it to be highly unfair for us to close PRs
>>>>>>>>> due to
>>>>>>>>> > >>>>> inactivity,
>>>>>>>>> > >>>>>>>>> when the primary cause has been /our /own inactivity.
>>>>>>>>> > >>>>>>>>> If a PR is opened and the first comment comes in 3
>>>>>>>>> months late,
>>>>>>>>> > >>>> then I
>>>>>>>>> > >>>>>>>>> don't blame the contributor for not responding.
>>>>>>>>> > >>>>>>>>> IMO we owe it to the contributor to evaluate their PR,
>>>>>>>>> and if
>>>>>>>>> > >>>>> necessary
>>>>>>>>> > >>>>>>>>> bring it to a merge-able state (to a certain extend).
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> There's also the fact that closing these PRs outright
>>>>>>>>> would
>>>>>>>>> waste a
>>>>>>>>> > >>>>> lot
>>>>>>>>> > >>>>>>>>> of
>>>>>>>>> > >>>>>>>>> good contributions.
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> Finally, this solution is overkill for what we want to
>>>>>>>>> achieve.
>>>>>>>>> If
>>>>>>>>> > >>>> we
>>>>>>>>> > >>>>>>>>> want
>>>>>>>>> > >>>>>>>>> to make it easier to find PRs to review all we need is
>>>>>>>>> > >>>>>>>>> GitBox integration and tagging or PRs. That's it. We
>>>>>>>>> could have
>>>>>>>>> a
>>>>>>>>> > >>>>> /fully
>>>>>>>>> > >>>>>>>>> /tagged PR list /tomorrow/, if we really wanted to.
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> On 15.05.2018 05:10, Ted Yu wrote:
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>>> bq. this pull request requires a review, please simply
>>>>>>>>> write any
>>>>>>>>> > >>>>>>>>>> comment.
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>> Shouldn't the wording of such comment be known before
>>>>>>>>> hand ?
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>> Otherwise pull request waiting for committers' review
>>>>>>>>> may be
>>>>>>>>> > >>>>>>>>>> mis-classified.
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>> Cheers
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>> On Mon, May 14, 2018 at 7:59 PM, blues zheng <
>>>>>>>>> kisimple@163.com>
>>>>>>>>> > >>>>> wrote:
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>> +1 for the proposal.
>>>>>>>>> > >>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> Best,
>>>>>>>>> > >>>>>>>>>>> blues
>>>>>>>>> > >>>>>>>>>>> On 05/14/2018 20:58, Ufuk Celebi wrote:
>>>>>>>>> > >>>>>>>>>>> Hey Piotr,
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> thanks for bringing this up. I really like this
>>>>>>>>> proposal and
>>>>>>>>> also
>>>>>>>>> > >>>>> saw
>>>>>>>>> > >>>>>>>>>>> it work successfully at other projects. So +1 from
>>>>>>>>> my side.
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> - I like the approach with a notification one week
>>>>>>>>> before
>>>>>>>>> > >>>>>>>>>>> automatically closing the PR
>>>>>>>>> > >>>>>>>>>>> - I think a bot will the best option as these kinds
>>>>>>>>> of things
>>>>>>>>> are
>>>>>>>>> > >>>>>>>>>>> usually followed enthusiastically in the beginning
>>>>>>>>> but
>>>>>>>>> eventually
>>>>>>>>> > >>>>>>>>>>> loose traction
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> We can enable better integration with GitHub by
>>>>>>>>> using ASF
>>>>>>>>> GitBox
>>>>>>>>> > >>>>>>>>>>> (https://gitbox.apache.org/setup/) but we should
>>>>>>>>> discuss that
>>>>>>>>> in
>>>>>>>>> > >>>> a
>>>>>>>>> > >>>>>>>>>>> separate thread.
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> – Ufuk
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> On Mon, May 14, 2018 at 12:04 PM, Piotr Nowojski
>>>>>>>>> > >>>>>>>>>>> <pi...@data-artisans.com> wrote:
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> Hey,
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> We have lots of open pull requests and quite some
>>>>>>>>> of them are
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> stale/abandoned/inactive. Often such old PRs are
>>>>>>>>> impossible
>>>>>>>>> to
>>>>>>>>> > >>>>> merge
>>>>>>>>> > >>>>>>>>>>> due
>>>>>>>>> > >>>>>>>>>>> to
>>>>>>>>> > >>>>>>>>>>> conflicts and it’s easier to just abandon and
>>>>>>>>> rewrite them.
>>>>>>>>> > >>>>> Especially
>>>>>>>>> > >>>>>>>>>>> there are some PRs which original contributor
>>>>>>>>> created long
>>>>>>>>> time
>>>>>>>>> > >>>> ago,
>>>>>>>>> > >>>>>>>>>>> someone else wrote some comments/review and… that’s
>>>>>>>>> about it.
>>>>>>>>> > >>>>> Original
>>>>>>>>> > >>>>>>>>>>> contributor never shown up again to respond to the
>>>>>>>>> comments.
>>>>>>>>> > >>>>>>>>>>> Regardless
>>>>>>>>> > >>>>>>>>>>> of
>>>>>>>>> > >>>>>>>>>>> the reason such PRs are clogging the GitHub, making
>>>>>>>>> it
>>>>>>>>> difficult
>>>>>>>>> > >>>> to
>>>>>>>>> > >>>>>>>>>>> keep
>>>>>>>>> > >>>>>>>>>>> track of things and making it almost impossible to
>>>>>>>>> find a
>>>>>>>>> little
>>>>>>>>> > >>>> bit
>>>>>>>>> > >>>>>>>>>>> old
>>>>>>>>> > >>>>>>>>>>> (for example 3+ months) PRs that are still valid and
>>>>>>>>> waiting
>>>>>>>>> for
>>>>>>>>> > >>>>>>>>>>> reviews.
>>>>>>>>> > >>>>>>>>>>> To do something like that, one would have to dig
>>>>>>>>> through tens
>>>>>>>>> or
>>>>>>>>> > >>>>>>>>>>> hundreds
>>>>>>>>> > >>>>>>>>>>> of abandoned PRs.
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> What I would like to propose is to agree on some
>>>>>>>>> inactivity
>>>>>>>>> dead
>>>>>>>>> > >>>>> line,
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> lets say 3 months. After crossing such deadline,
>>>>>>>>> PRs should
>>>>>>>>> be
>>>>>>>>> > >>>>>>>>>>> marked/commented as “stale”, with information like:
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> “This pull request has been marked as stale due to 3
>>>>>>>>> months of
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> inactivity. It will be closed in 1 week if no
>>>>>>>>> further
>>>>>>>>> activity
>>>>>>>>> > >>>>>>>>>>> occurs. If
>>>>>>>>> > >>>>>>>>>>> you think that’s incorrect or this pull request
>>>>>>>>> requires a
>>>>>>>>> > >> review,
>>>>>>>>> > >>>>>>>>>>> please
>>>>>>>>> > >>>>>>>>>>> simply write any comment.”
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> Either we could just agree on such policy and
>>>>>>>>> enforce it
>>>>>>>>> manually
>>>>>>>>> > >>>>>>>>>>>> (maybe
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> with some simple tooling, like a simple script to
>>>>>>>>> list
>>>>>>>>> inactive
>>>>>>>>> > >>>>> PRs -
>>>>>>>>> > >>>>>>>>>>> seems
>>>>>>>>> > >>>>>>>>>>> like couple of lines in python by using PyGithub) or
>>>>>>>>> we could
>>>>>>>>> > >>>> think
>>>>>>>>> > >>>>>>>>>>> about
>>>>>>>>> > >>>>>>>>>>> automating this action. There are some bots that do
>>>>>>>>> exactly
>>>>>>>>> this
>>>>>>>>> > >>>>> (like
>>>>>>>>> > >>>>>>>>>>> this
>>>>>>>>> > >>>>>>>>>>> one: https://github.com/probot/stale <
>>>>>>>>> https://github.com/probot/
>>>>>>>>> > >>>>> stale
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> ),
>>>>>>>>> > >>>>>>>>>>> but probably they would need to be adopted to
>>>>>>>>> limitations of
>>>>>>>>> our
>>>>>>>>> > >>>>>>>>>>> Apache
>>>>>>>>> > >>>>>>>>>>> repository (we can not add labels and we can not
>>>>>>>>> close the PRs
>>>>>>>>> > >> via
>>>>>>>>> > >>>>>>>>>>> GitHub).
>>>>>>>>> > >>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>> What do you think about it?
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>> Piotrek
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>>>>
>>>>>>>>> > >>>>>>>>>
>>>>>>>>> > >>>>>>>
>>>>>>>>> > >>>>>>
>>>>>>>>> > >>>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>
>>>>>>>>> > >>
>>>>>>>>>
>>>>>>>>
Re: Closing (automatically?) inactive pull requests
Posted by Kenneth Knowles <kl...@google.com>.
Ismael had mentioned to vote on this, so let's do that now to make sure no
one missed this discussion.
On Thu, May 31, 2018 at 9:33 PM Alan Myrvold <am...@google.com> wrote:
> Thanks. I can look into adding the stale.yaml file for old pull requests/
>
> On Thu, May 31, 2018 at 8:07 PM Kenneth Knowles <kl...@google.com> wrote:
>
>> Update: you brought the information needed, and it is now enabled. Thanks
>> for the follow-through!
>>
>> Since you dug into probot's details, I took the liberty of assigning
>> BEAM-4423 to you, in case throwing together the needed configs is fresh in
>> your mind and you are in the mood to continue. (if not, pass the hot
>> potato, back to me is OK too :-)
>>
>> Kenn
>>
>> On Thu, May 31, 2018 at 4:50 PM Alan Myrvold <am...@google.com> wrote:
>>
>>> INFRA-16589 got closed asking to clarify that the probot-stale app would
>>> not have permissions to merge automatically.
>>> From my reading of the permissions documentation, it would not. I added
>>> a comment to INFRA-16589
>>>
>>> On Tue, May 29, 2018 at 10:05 AM Lukasz Cwik <lc...@google.com> wrote:
>>>
>>>> I opened up INFRA-16589
>>>> <https://issues.apache.org/jira/browse/INFRA-16589> for Apache infra
>>>> to install Stale but they denied a similar request INFRA-16394
>>>> <https://issues.apache.org/jira/browse/INFRA-16394> because of
>>>> permissions issues. I tried clarifying the permissions requested.
>>>>
>>>> On Tue, May 29, 2018 at 9:39 AM Scott Wegner <sw...@google.com>
>>>> wrote:
>>>>
>>>>> +1. I've opened BEAM-4423 to capture the discussion here:
>>>>> https://issues.apache.org/jira/browse/BEAM-4423
>>>>>
>>>>> On Thu, May 24, 2018 at 5:34 PM Chamikara Jayalath <
>>>>> chamikara@google.com> wrote:
>>>>>
>>>>>> +1 for automatically closing. Currently, contribution guide mentions
>>>>>> that pull requests without responses to actionable comments become stale
>>>>>> (and closed) after two months but last I checked there were many pull
>>>>>> requests that met this criteria but had not been closed after many months.
>>>>>> So seems like humans are reluctant to act on the established policy :)
>>>>>>
>>>>>> - Cham
>>>>>>
>>>>>> On Wed, May 23, 2018 at 11:25 AM Kenneth Knowles <kl...@google.com>
>>>>>> wrote:
>>>>>>
>>>>>>> That makes sense, to just focus on Beam's decision. It seems the
>>>>>>> tool is already built. I thought we just had to deploy it, but maybe not
>>>>>>> even that, if we can just activate it: https://github.com/apps/stale
>>>>>>>
>>>>>>> Kenn
>>>>>>>
>>>>>>> On Wed, May 23, 2018 at 9:31 AM Ismaël Mejía <ie...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Given that reaching consensus in both communities seems like a
>>>>>>>> harder task
>>>>>>>> than just deciding our policy. in the Beam side Why don't we just
>>>>>>>> go ahead
>>>>>>>> and vote around this + build the tool, and if the Flink guys are
>>>>>>>> interested
>>>>>>>> they can take it, no?
>>>>>>>>
>>>>>>>> in the future we can share that code.
>>>>>>>> On Wed, May 16, 2018 at 3:31 PM Piotr Nowojski <
>>>>>>>> piotr@data-artisans.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> > The question is what would such tool offer on top of over a
>>>>>>>> Github’s view
>>>>>>>> of PR sorted by “least recently updated”:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> https://github.com/apache/flink/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc
>>>>>>>> <
>>>>>>>> https://github.com/apache/flink/pulls?q=is:pr+is:open+sort:updated-asc
>>>>>>>> >
>>>>>>>>
>>>>>>>> > ? Maybe it would be good enough to have a policy about waiting x
>>>>>>>> months/weeks for a contributor to respond. If he doesn’t, we either
>>>>>>>> take
>>>>>>>> over PR we or close it. Having “clean” view of oldest PRs would be
>>>>>>>> beneficial for reviewing PRs in ~FIFO order as part of community
>>>>>>>> work.
>>>>>>>>
>>>>>>>> > Having
>>>>>>>>
>>>>>>>> > > On 16 May 2018, at 10:57, Fabian Hueske <fh...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> > >
>>>>>>>> > > Hi,
>>>>>>>> > >
>>>>>>>> > > I'm not objecting closing stale PRs.
>>>>>>>> > > We have quite a few PRs with very little chance of being merged
>>>>>>>> and I
>>>>>>>> would
>>>>>>>> > > certainly appreciate cleaning up those.
>>>>>>>> > > However, I think we should not automate closing PRs for the
>>>>>>>> reasons I
>>>>>>>> gave
>>>>>>>> > > before.
>>>>>>>> > >
>>>>>>>> > > A tool that reminds us of state PRs as proposed by Till seems
>>>>>>>> like a
>>>>>>>> good
>>>>>>>> > > idea and might actually help to re-adjust priorities from time
>>>>>>>> to time.
>>>>>>>> > >
>>>>>>>> > > @Yazdan: Right now there is no assignment happening. Committers
>>>>>>>> decide
>>>>>>>> > > which PRs they want to review and merge.
>>>>>>>> > >
>>>>>>>> > > Best, Fabian
>>>>>>>> > >
>>>>>>>> > > 2018-05-16 4:26 GMT+02:00 Yaz Sh <ya...@gmail.com>:
>>>>>>>> > >
>>>>>>>> > >> I have questions in this regard (you guys might have addressed
>>>>>>>> it in
>>>>>>>> this
>>>>>>>> > >> email chain):
>>>>>>>> > >> how PRs get assigned to a reviewer apart of a contributor tag
>>>>>>>> someone?
>>>>>>>> > >> what if PR never gets a reviewer attention and it became
>>>>>>>> in-active due
>>>>>>>> to
>>>>>>>> > >> long review respond? should Bot assign a reviewer to a PR
>>>>>>>> based on
>>>>>>>> > >> reviewers interest (i.e., defined via tags) and notify he/she
>>>>>>>> if PR is
>>>>>>>> > >> waiting for review?
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>> > >> Cheers,
>>>>>>>> > >> /Yazdan
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>> > >>> On May 15, 2018, at 12:06 PM, Thomas Weise <th...@apache.org>
>>>>>>>> wrote:
>>>>>>>> > >>>
>>>>>>>> > >>> I like Till's proposal to notify the participants on the PR
>>>>>>>> to PTAL.
>>>>>>>> But
>>>>>>>> > >> I
>>>>>>>> > >>> would also suggest to auto-close when no action is taken,
>>>>>>>> with a
>>>>>>>> friendly
>>>>>>>> > >>> reminder that PRs can be reopened anytime.
>>>>>>>> > >>>
>>>>>>>> > >>> The current situation with 350 open PRs may send a signal to
>>>>>>>> contributors
>>>>>>>> > >>> that it may actually be too much hassle to get a change
>>>>>>>> committed in
>>>>>>>> > >> Flink.
>>>>>>>> > >>> Since the count keeps on rising, this is also not a past
>>>>>>>> problem.
>>>>>>>> Pruning
>>>>>>>> > >>> inactive PRs may help, that will also give a more accurate
>>>>>>>> picture of
>>>>>>>> the
>>>>>>>> > >>> lack of review bandwidth, if that is the root cause.
>>>>>>>> > >>>
>>>>>>>> > >>> Thanks,
>>>>>>>> > >>> Thomas
>>>>>>>> > >>>
>>>>>>>> > >>>
>>>>>>>> > >>>
>>>>>>>> > >>>
>>>>>>>> > >>>
>>>>>>>> > >>> On Tue, May 15, 2018 at 8:24 AM, Ted Yu <yu...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> > >>>
>>>>>>>> > >>>> How does the bot decide whether the PR is waiting for
>>>>>>>> reviews or is
>>>>>>>> > >> being
>>>>>>>> > >>>> abandoned by contributor ?
>>>>>>>> > >>>>
>>>>>>>> > >>>> How about letting the bot count the number of times
>>>>>>>> contributor pings
>>>>>>>> > >>>> committer(s) for review ?
>>>>>>>> > >>>> When unanswered ping count crosses some threshold, say 3,
>>>>>>>> the bot
>>>>>>>> > >> publishes
>>>>>>>> > >>>> the JIRA and PR somewhere.
>>>>>>>> > >>>>
>>>>>>>> > >>>> Cheers
>>>>>>>> > >>>>
>>>>>>>> > >>>> On Tue, May 15, 2018 at 8:19 AM, Till Rohrmann <
>>>>>>>> trohrmann@apache.org>
>>>>>>>> > >>>> wrote:
>>>>>>>> > >>>>
>>>>>>>> > >>>>> I'm a bit torn here because I can see the pros and cons for
>>>>>>>> both
>>>>>>>> sides.
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> Maybe a compromise could be to not have a closing but a
>>>>>>>> monitoring
>>>>>>>> bot
>>>>>>>> > >>>>> which notifies us about inactive PRs. This could then
>>>>>>>> trigger an
>>>>>>>> > >>>>> investigation of the underlying problem and ultimately lead
>>>>>>>> to a
>>>>>>>> > >>>> conscious
>>>>>>>> > >>>>> decision to close or keep the PR open. As such it is not
>>>>>>>> strictly
>>>>>>>> > >>>> necessary
>>>>>>>> > >>>>> to have such a bot but it would at least remind us to make a
>>>>>>>> decision
>>>>>>>> > >>>> about
>>>>>>>> > >>>>> older PRs with no activity.
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> Cheers,
>>>>>>>> > >>>>> Till
>>>>>>>> > >>>>>
>>>>>>>> > >>>>> On Tue, May 15, 2018 at 3:26 PM, Chesnay Schepler <
>>>>>>>> chesnay@apache.org>
>>>>>>>> > >>>>> wrote:
>>>>>>>> > >>>>>
>>>>>>>> > >>>>>> /So far I did it twice for older PRs. In both cases I
>>>>>>>> didn’t get
>>>>>>>> any
>>>>>>>> > >>>>>> response and I even forgot in which PRs I had asked this
>>>>>>>> question,
>>>>>>>> so
>>>>>>>> > >>>>> now I
>>>>>>>> > >>>>>> can not even close them :S/
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> To be honest this sounds more like an issue with how your
>>>>>>>> organize
>>>>>>>> > >> your
>>>>>>>> > >>>>>> work. No amount of closing PRs can fix that.
>>>>>>>> > >>>>>> With GitBox we can assign reviewers to a PR, but I'm not
>>>>>>>> sure
>>>>>>>> whether
>>>>>>>> > >>>> it
>>>>>>>> > >>>>>> only allows committers to assign people.
>>>>>>>> > >>>>>> Bookmarks or text files might help as well./
>>>>>>>> > >>>>>> /
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> /Regarding only 30% blocked on contributor. I wonder what
>>>>>>>> would be
>>>>>>>> > >> this
>>>>>>>> > >>>>>> number if we tried to ask in the rest of old PRs “Hey, are
>>>>>>>> you
>>>>>>>> still
>>>>>>>> > >>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>>> waiting for a
>>>>>>>> > >>>>>> reviewer for X months, it doesn’t mean that’s not
>>>>>>>> abandoned. Even
>>>>>>>> if
>>>>>>>> > >> it
>>>>>>>> > >>>>>> doesn’t, reducing our overhead by those 30% of the PRs is
>>>>>>>> something./
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> No doubt the number would be higher if we were to go back,
>>>>>>>> but as i
>>>>>>>> > >>>>>> explained earlier that is not a reason to close it. If a
>>>>>>>> PR is
>>>>>>>> > >>>> abandoned
>>>>>>>> > >>>>>> because we messed up we should still /try /to get it in.
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> /This is kind of whole point of what I was proposing. If
>>>>>>>> the PR
>>>>>>>> author
>>>>>>>> > >>>> is
>>>>>>>> > >>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>>> timeout,
>>>>>>>> > >> that’s
>>>>>>>> > >>>>>> great. Gives us even more sense of urgency to review it./
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> Unfortunately knowing that it is more urgent is
>>>>>>>> irrelevant, as we
>>>>>>>> > >>>>>> currently don't have the manpower to review them. Reviving
>>>>>>>> them now
>>>>>>>> > >>>> would
>>>>>>>> > >>>>>> serve no purpose. The alternative is to wait until more
>>>>>>>> people
>>>>>>>> become
>>>>>>>> > >>>>>> active reviewers.
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> /As a last resort, closing PR after timeout is not the end
>>>>>>>> of the
>>>>>>>> > >>>> world.
>>>>>>>> > >>>>>> It always can be reopened./
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> Let's be realistic here, it will not be reopened.
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>> On 15.05.2018 14:21, Piotr Nowojski wrote:
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>>> I agree that we have other, even more important, problems
>>>>>>>> with
>>>>>>>> > >>>> reviewing
>>>>>>>> > >>>>>>> PR and community, but that shouldn’t block us from trying
>>>>>>>> to
>>>>>>>> clean up
>>>>>>>> > >>>>>>> things a little bit and minimise the effort needed for
>>>>>>>> reviewing
>>>>>>>> PRs.
>>>>>>>> > >>>>> Now
>>>>>>>> > >>>>>>> before reviewing/picking older PRs I had to ask this
>>>>>>>> “Hey, are you
>>>>>>>> > >>>> still
>>>>>>>> > >>>>>>> interested in merging this?” manually and wait for the
>>>>>>>> response.
>>>>>>>> If
>>>>>>>> > >> it
>>>>>>>> > >>>>>>> doesn’t come, I have to remember to go back and close PR,
>>>>>>>> which I
>>>>>>>> of
>>>>>>>> > >>>>> course
>>>>>>>> > >>>>>>> forget to do. Bah, so far I did it twice for older PRs.
>>>>>>>> In both
>>>>>>>> cases
>>>>>>>> > >>>> I
>>>>>>>> > >>>>>>> didn’t get any response and I even forgot in which PRs I
>>>>>>>> had asked
>>>>>>>> > >>>> this
>>>>>>>> > >>>>>>> question, so now I can not even close them :S Wasted
>>>>>>>> effort and
>>>>>>>> > >> wasted
>>>>>>>> > >>>>> time
>>>>>>>> > >>>>>>> on context switching for me and for everyone else that
>>>>>>>> will have
>>>>>>>> to
>>>>>>>> > >>>>> scroll
>>>>>>>> > >>>>>>> pass or look on those PR to check their status.
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>> Keep in mind that I am not proposing to close the PR
>>>>>>>> automatically
>>>>>>>> > >>>>>>> straight on after 3 months of inactivity. Only after
>>>>>>>> asking a
>>>>>>>> > >> question
>>>>>>>> > >>>>>>> whether original contributor is still there and he is
>>>>>>>> interested
>>>>>>>> in
>>>>>>>> > >>>> the
>>>>>>>> > >>>>> PR
>>>>>>>> > >>>>>>> to be reviewed.
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>> for Flink 1.5, I merged a contribution from PR #1990
>>>>>>>> after it was
>>>>>>>> > >>>>>>>> requested a few times by users.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>> This is kind of whole point of what I was proposing. If
>>>>>>>> the PR
>>>>>>>> author
>>>>>>>> > >>>> is
>>>>>>>> > >>>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>>> timeout,
>>>>>>>> > >>>> that’s
>>>>>>>> > >>>>>>> great. Gives us even more sense of urgency to review it.
>>>>>>>> On the
>>>>>>>> other
>>>>>>>> > >>>>> hand
>>>>>>>> > >>>>>>> if there is no response (no interest from the author for
>>>>>>>> whatever
>>>>>>>> the
>>>>>>>> > >>>>>>> reasons) and nobody so far has picked this PR to
>>>>>>>> review/merge,
>>>>>>>> what’s
>>>>>>>> > >>>>> the
>>>>>>>> > >>>>>>> point of keeping such PR open? As a last resort, closing
>>>>>>>> PR after
>>>>>>>> > >>>>> timeout
>>>>>>>> > >>>>>>> is not the end of the world. It always can be reopened.
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>> Regarding only 30% blocked on contributor. I wonder what
>>>>>>>> would be
>>>>>>>> > >> this
>>>>>>>> > >>>>>>> number if we tried to ask in the rest of old PRs “Hey,
>>>>>>>> are you
>>>>>>>> still
>>>>>>>> > >>>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>>> waiting for a
>>>>>>>> > >>>>> reviewer
>>>>>>>> > >>>>>>> for X months, it doesn’t mean that’s not abandoned. Even
>>>>>>>> if it
>>>>>>>> > >>>> doesn’t,
>>>>>>>> > >>>>>>> reducing our overhead by those 30% of the PRs is
>>>>>>>> something.
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>> Piotrek
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>> On 15 May 2018, at 10:10, Fabian Hueske <
>>>>>>>> fhueske@gmail.com> wrote:
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> I'm with Chesnay on this issue.
>>>>>>>> > >>>>>>>> Stale PRs, i.e., a PR where a contributor becomes
>>>>>>>> inactive, are
>>>>>>>> one
>>>>>>>> > >>>> of
>>>>>>>> > >>>>>>>> our
>>>>>>>> > >>>>>>>> smallest issues, IMO.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> There are more reasons for the high number of PRs.
>>>>>>>> > >>>>>>>> * Lack of timely reviews.
>>>>>>>> > >>>>>>>> * Not eagerly closing PRs that have no or very little
>>>>>>>> chance of
>>>>>>>> > >> being
>>>>>>>> > >>>>>>>> merged. Common reasons are
>>>>>>>> > >>>>>>>> 1) lack of interest in the feature by committers,
>>>>>>>> > >>>>>>>> 2) too extensive changes and hence time consuming
>>>>>>>> reviews, or
>>>>>>>> > >>>>>>>> 3) bad quality.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> For 1), there are lots of older JIRA issues, that have
>>>>>>>> low
>>>>>>>> priority
>>>>>>>> > >>>> but
>>>>>>>> > >>>>>>>> which are picked up by contributors. In the contribution
>>>>>>>> guidelines
>>>>>>>> > >>>> we
>>>>>>>> > >>>>>>>> ask
>>>>>>>> > >>>>>>>> contributors to let us know when they want to work on an
>>>>>>>> issue.
>>>>>>>> So
>>>>>>>> > >>>> far
>>>>>>>> > >>>>>>>> our
>>>>>>>> > >>>>>>>> attitude has been, yes sure go ahead. I've seen very
>>>>>>>> little
>>>>>>>> attempts
>>>>>>>> > >>>> of
>>>>>>>> > >>>>>>>> warning somebody to work on issues that won't be easily
>>>>>>>> merged.
>>>>>>>> > >>>>>>>> Once a PR has been opened, we should also be honest and
>>>>>>>> let
>>>>>>>> > >>>>> contributors
>>>>>>>> > >>>>>>>> know that it has no chance or might take a while to get
>>>>>>>> reviewed.
>>>>>>>> > >>>>>>>> For 2) this is typically not so much of an issue if the
>>>>>>>> feature
>>>>>>>> is
>>>>>>>> > >>>>>>>> interesting. However, if 1) and 2) meet, chances to get
>>>>>>>> a change
>>>>>>>> in
>>>>>>>> > >>>>> drop
>>>>>>>> > >>>>>>>> even more.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> A common "strategy" to deal with PRs that fall into 1),
>>>>>>>> 2), or
>>>>>>>> 3) is
>>>>>>>> > >>>> to
>>>>>>>> > >>>>>>>> not
>>>>>>>> > >>>>>>>> look at them or giving a shallow review.
>>>>>>>> > >>>>>>>> Of course, contributors become unresponsive if we don't
>>>>>>>> look at
>>>>>>>> > >> their
>>>>>>>> > >>>>> PRs
>>>>>>>> > >>>>>>>> for weeks or months. But that's not their fault.
>>>>>>>> > >>>>>>>> Instead, I think we should be honest and communicate the
>>>>>>>> chances
>>>>>>>> of
>>>>>>>> > >> a
>>>>>>>> > >>>>> PR
>>>>>>>> > >>>>>>>> more clearly.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> Browsing over the list of open PRs, I feel that most
>>>>>>>> older PRs
>>>>>>>> fall
>>>>>>>> > >>>>> into
>>>>>>>> > >>>>>>>> the category of low-priority (or even unwanted) features.
>>>>>>>> > >>>>>>>> Bug fixes or features that the committers care about
>>>>>>>> usually
>>>>>>>> make it
>>>>>>>> > >>>>> into
>>>>>>>> > >>>>>>>> the code base.
>>>>>>>> > >>>>>>>> In case a contributor becomes inactive, committers often
>>>>>>>> take
>>>>>>>> over
>>>>>>>> > >> an
>>>>>>>> > >>>>>>>> push
>>>>>>>> > >>>>>>>> a contribution over the line.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> So, I do not support an auto-close mechanism.
>>>>>>>> > >>>>>>>> I think a better way to address the issue is better
>>>>>>>> communication,
>>>>>>>> > >>>> more
>>>>>>>> > >>>>>>>> eagerly closing PRs with no chance, and putting more
>>>>>>>> reviewing
>>>>>>>> > >>>> effort.
>>>>>>>> > >>>>>>>> IMO, we should only eagerly close PRs that have no
>>>>>>>> chance of
>>>>>>>> being
>>>>>>>> > >>>>>>>> merged.
>>>>>>>> > >>>>>>>> PRs with low-prio features might be picked up later (for
>>>>>>>> Flink
>>>>>>>> 1.5,
>>>>>>>> > >> I
>>>>>>>> > >>>>>>>> merged a contribution from PR #1990 after it was
>>>>>>>> requested a few
>>>>>>>> > >>>> times
>>>>>>>> > >>>>> by
>>>>>>>> > >>>>>>>> users).
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> However, I think we could do a pass over the oldest PRs
>>>>>>>> and
>>>>>>>> check if
>>>>>>>> > >>>> we
>>>>>>>> > >>>>>>>> can
>>>>>>>> > >>>>>>>> close a bunch.
>>>>>>>> > >>>>>>>> There are quite a few contributions (many for flink-ml)
>>>>>>>> that I
>>>>>>>> don't
>>>>>>>> > >>>>> see
>>>>>>>> > >>>>>>>> a
>>>>>>>> > >>>>>>>> chance for getting merged.
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> Best, Fabian
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> -
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> 2018-05-15 9:13 GMT+02:00 Chesnay Schepler <
>>>>>>>> chesnay@apache.org>:
>>>>>>>> > >>>>>>>>
>>>>>>>> > >>>>>>>> -1
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> For clarification (since the original mail indicates
>>>>>>>> otherwise),
>>>>>>>> > >> the
>>>>>>>> > >>>>>>>>> number of pull requests that this would affect is
>>>>>>>> fairly small.
>>>>>>>> > >>>>>>>>> Only about 25-30% of all open PRs are blocked on the
>>>>>>>> contributor,
>>>>>>>> > >>>> the
>>>>>>>> > >>>>>>>>> remaining ones are actually blocked on the review.
>>>>>>>> > >>>>>>>>> Thus is reject the premise that one has to search
>>>>>>>> through that
>>>>>>>> many
>>>>>>>> > >>>>> PRs
>>>>>>>> > >>>>>>>>> to
>>>>>>>> > >>>>>>>>> find something to review, there is plenty.
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> I believe it to be highly unfair for us to close PRs
>>>>>>>> due to
>>>>>>>> > >>>>> inactivity,
>>>>>>>> > >>>>>>>>> when the primary cause has been /our /own inactivity.
>>>>>>>> > >>>>>>>>> If a PR is opened and the first comment comes in 3
>>>>>>>> months late,
>>>>>>>> > >>>> then I
>>>>>>>> > >>>>>>>>> don't blame the contributor for not responding.
>>>>>>>> > >>>>>>>>> IMO we owe it to the contributor to evaluate their PR,
>>>>>>>> and if
>>>>>>>> > >>>>> necessary
>>>>>>>> > >>>>>>>>> bring it to a merge-able state (to a certain extend).
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> There's also the fact that closing these PRs outright
>>>>>>>> would
>>>>>>>> waste a
>>>>>>>> > >>>>> lot
>>>>>>>> > >>>>>>>>> of
>>>>>>>> > >>>>>>>>> good contributions.
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> Finally, this solution is overkill for what we want to
>>>>>>>> achieve.
>>>>>>>> If
>>>>>>>> > >>>> we
>>>>>>>> > >>>>>>>>> want
>>>>>>>> > >>>>>>>>> to make it easier to find PRs to review all we need is
>>>>>>>> > >>>>>>>>> GitBox integration and tagging or PRs. That's it. We
>>>>>>>> could have
>>>>>>>> a
>>>>>>>> > >>>>> /fully
>>>>>>>> > >>>>>>>>> /tagged PR list /tomorrow/, if we really wanted to.
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> On 15.05.2018 05:10, Ted Yu wrote:
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>>> bq. this pull request requires a review, please simply
>>>>>>>> write any
>>>>>>>> > >>>>>>>>>> comment.
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>> Shouldn't the wording of such comment be known before
>>>>>>>> hand ?
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>> Otherwise pull request waiting for committers' review
>>>>>>>> may be
>>>>>>>> > >>>>>>>>>> mis-classified.
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>> Cheers
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>> On Mon, May 14, 2018 at 7:59 PM, blues zheng <
>>>>>>>> kisimple@163.com>
>>>>>>>> > >>>>> wrote:
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>> +1 for the proposal.
>>>>>>>> > >>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> Best,
>>>>>>>> > >>>>>>>>>>> blues
>>>>>>>> > >>>>>>>>>>> On 05/14/2018 20:58, Ufuk Celebi wrote:
>>>>>>>> > >>>>>>>>>>> Hey Piotr,
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> thanks for bringing this up. I really like this
>>>>>>>> proposal and
>>>>>>>> also
>>>>>>>> > >>>>> saw
>>>>>>>> > >>>>>>>>>>> it work successfully at other projects. So +1 from my
>>>>>>>> side.
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> - I like the approach with a notification one week
>>>>>>>> before
>>>>>>>> > >>>>>>>>>>> automatically closing the PR
>>>>>>>> > >>>>>>>>>>> - I think a bot will the best option as these kinds
>>>>>>>> of things
>>>>>>>> are
>>>>>>>> > >>>>>>>>>>> usually followed enthusiastically in the beginning but
>>>>>>>> eventually
>>>>>>>> > >>>>>>>>>>> loose traction
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> We can enable better integration with GitHub by using
>>>>>>>> ASF
>>>>>>>> GitBox
>>>>>>>> > >>>>>>>>>>> (https://gitbox.apache.org/setup/) but we should
>>>>>>>> discuss that
>>>>>>>> in
>>>>>>>> > >>>> a
>>>>>>>> > >>>>>>>>>>> separate thread.
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> – Ufuk
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> On Mon, May 14, 2018 at 12:04 PM, Piotr Nowojski
>>>>>>>> > >>>>>>>>>>> <pi...@data-artisans.com> wrote:
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> Hey,
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> We have lots of open pull requests and quite some of
>>>>>>>> them are
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> stale/abandoned/inactive. Often such old PRs are
>>>>>>>> impossible
>>>>>>>> to
>>>>>>>> > >>>>> merge
>>>>>>>> > >>>>>>>>>>> due
>>>>>>>> > >>>>>>>>>>> to
>>>>>>>> > >>>>>>>>>>> conflicts and it’s easier to just abandon and rewrite
>>>>>>>> them.
>>>>>>>> > >>>>> Especially
>>>>>>>> > >>>>>>>>>>> there are some PRs which original contributor created
>>>>>>>> long
>>>>>>>> time
>>>>>>>> > >>>> ago,
>>>>>>>> > >>>>>>>>>>> someone else wrote some comments/review and… that’s
>>>>>>>> about it.
>>>>>>>> > >>>>> Original
>>>>>>>> > >>>>>>>>>>> contributor never shown up again to respond to the
>>>>>>>> comments.
>>>>>>>> > >>>>>>>>>>> Regardless
>>>>>>>> > >>>>>>>>>>> of
>>>>>>>> > >>>>>>>>>>> the reason such PRs are clogging the GitHub, making it
>>>>>>>> difficult
>>>>>>>> > >>>> to
>>>>>>>> > >>>>>>>>>>> keep
>>>>>>>> > >>>>>>>>>>> track of things and making it almost impossible to
>>>>>>>> find a
>>>>>>>> little
>>>>>>>> > >>>> bit
>>>>>>>> > >>>>>>>>>>> old
>>>>>>>> > >>>>>>>>>>> (for example 3+ months) PRs that are still valid and
>>>>>>>> waiting
>>>>>>>> for
>>>>>>>> > >>>>>>>>>>> reviews.
>>>>>>>> > >>>>>>>>>>> To do something like that, one would have to dig
>>>>>>>> through tens
>>>>>>>> or
>>>>>>>> > >>>>>>>>>>> hundreds
>>>>>>>> > >>>>>>>>>>> of abandoned PRs.
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> What I would like to propose is to agree on some
>>>>>>>> inactivity
>>>>>>>> dead
>>>>>>>> > >>>>> line,
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> lets say 3 months. After crossing such deadline, PRs
>>>>>>>> should
>>>>>>>> be
>>>>>>>> > >>>>>>>>>>> marked/commented as “stale”, with information like:
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> “This pull request has been marked as stale due to 3
>>>>>>>> months of
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> inactivity. It will be closed in 1 week if no further
>>>>>>>> activity
>>>>>>>> > >>>>>>>>>>> occurs. If
>>>>>>>> > >>>>>>>>>>> you think that’s incorrect or this pull request
>>>>>>>> requires a
>>>>>>>> > >> review,
>>>>>>>> > >>>>>>>>>>> please
>>>>>>>> > >>>>>>>>>>> simply write any comment.”
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> Either we could just agree on such policy and enforce
>>>>>>>> it
>>>>>>>> manually
>>>>>>>> > >>>>>>>>>>>> (maybe
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> with some simple tooling, like a simple script to
>>>>>>>> list
>>>>>>>> inactive
>>>>>>>> > >>>>> PRs -
>>>>>>>> > >>>>>>>>>>> seems
>>>>>>>> > >>>>>>>>>>> like couple of lines in python by using PyGithub) or
>>>>>>>> we could
>>>>>>>> > >>>> think
>>>>>>>> > >>>>>>>>>>> about
>>>>>>>> > >>>>>>>>>>> automating this action. There are some bots that do
>>>>>>>> exactly
>>>>>>>> this
>>>>>>>> > >>>>> (like
>>>>>>>> > >>>>>>>>>>> this
>>>>>>>> > >>>>>>>>>>> one: https://github.com/probot/stale <
>>>>>>>> https://github.com/probot/
>>>>>>>> > >>>>> stale
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> ),
>>>>>>>> > >>>>>>>>>>> but probably they would need to be adopted to
>>>>>>>> limitations of
>>>>>>>> our
>>>>>>>> > >>>>>>>>>>> Apache
>>>>>>>> > >>>>>>>>>>> repository (we can not add labels and we can not
>>>>>>>> close the PRs
>>>>>>>> > >> via
>>>>>>>> > >>>>>>>>>>> GitHub).
>>>>>>>> > >>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>> What do you think about it?
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>> Piotrek
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>>>>
>>>>>>>> > >>>>>>>>>
>>>>>>>> > >>>>>>>
>>>>>>>> > >>>>>>
>>>>>>>> > >>>>>
>>>>>>>> > >>>>
>>>>>>>> > >>
>>>>>>>> > >>
>>>>>>>>
>>>>>>>
Re: Closing (automatically?) inactive pull requests
Posted by Alan Myrvold <am...@google.com>.
Thanks. I can look into adding the stale.yaml file for old pull requests/
On Thu, May 31, 2018 at 8:07 PM Kenneth Knowles <kl...@google.com> wrote:
> Update: you brought the information needed, and it is now enabled. Thanks
> for the follow-through!
>
> Since you dug into probot's details, I took the liberty of assigning
> BEAM-4423 to you, in case throwing together the needed configs is fresh in
> your mind and you are in the mood to continue. (if not, pass the hot
> potato, back to me is OK too :-)
>
> Kenn
>
> On Thu, May 31, 2018 at 4:50 PM Alan Myrvold <am...@google.com> wrote:
>
>> INFRA-16589 got closed asking to clarify that the probot-stale app would
>> not have permissions to merge automatically.
>> From my reading of the permissions documentation, it would not. I added a
>> comment to INFRA-16589
>>
>> On Tue, May 29, 2018 at 10:05 AM Lukasz Cwik <lc...@google.com> wrote:
>>
>>> I opened up INFRA-16589
>>> <https://issues.apache.org/jira/browse/INFRA-16589> for Apache infra to
>>> install Stale but they denied a similar request INFRA-16394
>>> <https://issues.apache.org/jira/browse/INFRA-16394> because of
>>> permissions issues. I tried clarifying the permissions requested.
>>>
>>> On Tue, May 29, 2018 at 9:39 AM Scott Wegner <sw...@google.com> wrote:
>>>
>>>> +1. I've opened BEAM-4423 to capture the discussion here:
>>>> https://issues.apache.org/jira/browse/BEAM-4423
>>>>
>>>> On Thu, May 24, 2018 at 5:34 PM Chamikara Jayalath <
>>>> chamikara@google.com> wrote:
>>>>
>>>>> +1 for automatically closing. Currently, contribution guide mentions
>>>>> that pull requests without responses to actionable comments become stale
>>>>> (and closed) after two months but last I checked there were many pull
>>>>> requests that met this criteria but had not been closed after many months.
>>>>> So seems like humans are reluctant to act on the established policy :)
>>>>>
>>>>> - Cham
>>>>>
>>>>> On Wed, May 23, 2018 at 11:25 AM Kenneth Knowles <kl...@google.com>
>>>>> wrote:
>>>>>
>>>>>> That makes sense, to just focus on Beam's decision. It seems the tool
>>>>>> is already built. I thought we just had to deploy it, but maybe not even
>>>>>> that, if we can just activate it: https://github.com/apps/stale
>>>>>>
>>>>>> Kenn
>>>>>>
>>>>>> On Wed, May 23, 2018 at 9:31 AM Ismaël Mejía <ie...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Given that reaching consensus in both communities seems like a
>>>>>>> harder task
>>>>>>> than just deciding our policy. in the Beam side Why don't we just go
>>>>>>> ahead
>>>>>>> and vote around this + build the tool, and if the Flink guys are
>>>>>>> interested
>>>>>>> they can take it, no?
>>>>>>>
>>>>>>> in the future we can share that code.
>>>>>>> On Wed, May 16, 2018 at 3:31 PM Piotr Nowojski <
>>>>>>> piotr@data-artisans.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> > The question is what would such tool offer on top of over a
>>>>>>> Github’s view
>>>>>>> of PR sorted by “least recently updated”:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> https://github.com/apache/flink/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc
>>>>>>> <
>>>>>>> https://github.com/apache/flink/pulls?q=is:pr+is:open+sort:updated-asc
>>>>>>> >
>>>>>>>
>>>>>>> > ? Maybe it would be good enough to have a policy about waiting x
>>>>>>> months/weeks for a contributor to respond. If he doesn’t, we either
>>>>>>> take
>>>>>>> over PR we or close it. Having “clean” view of oldest PRs would be
>>>>>>> beneficial for reviewing PRs in ~FIFO order as part of community
>>>>>>> work.
>>>>>>>
>>>>>>> > Having
>>>>>>>
>>>>>>> > > On 16 May 2018, at 10:57, Fabian Hueske <fh...@gmail.com>
>>>>>>> wrote:
>>>>>>> > >
>>>>>>> > > Hi,
>>>>>>> > >
>>>>>>> > > I'm not objecting closing stale PRs.
>>>>>>> > > We have quite a few PRs with very little chance of being merged
>>>>>>> and I
>>>>>>> would
>>>>>>> > > certainly appreciate cleaning up those.
>>>>>>> > > However, I think we should not automate closing PRs for the
>>>>>>> reasons I
>>>>>>> gave
>>>>>>> > > before.
>>>>>>> > >
>>>>>>> > > A tool that reminds us of state PRs as proposed by Till seems
>>>>>>> like a
>>>>>>> good
>>>>>>> > > idea and might actually help to re-adjust priorities from time
>>>>>>> to time.
>>>>>>> > >
>>>>>>> > > @Yazdan: Right now there is no assignment happening. Committers
>>>>>>> decide
>>>>>>> > > which PRs they want to review and merge.
>>>>>>> > >
>>>>>>> > > Best, Fabian
>>>>>>> > >
>>>>>>> > > 2018-05-16 4:26 GMT+02:00 Yaz Sh <ya...@gmail.com>:
>>>>>>> > >
>>>>>>> > >> I have questions in this regard (you guys might have addressed
>>>>>>> it in
>>>>>>> this
>>>>>>> > >> email chain):
>>>>>>> > >> how PRs get assigned to a reviewer apart of a contributor tag
>>>>>>> someone?
>>>>>>> > >> what if PR never gets a reviewer attention and it became
>>>>>>> in-active due
>>>>>>> to
>>>>>>> > >> long review respond? should Bot assign a reviewer to a PR based
>>>>>>> on
>>>>>>> > >> reviewers interest (i.e., defined via tags) and notify he/she
>>>>>>> if PR is
>>>>>>> > >> waiting for review?
>>>>>>> > >>
>>>>>>> > >>
>>>>>>> > >> Cheers,
>>>>>>> > >> /Yazdan
>>>>>>> > >>
>>>>>>> > >>
>>>>>>> > >>> On May 15, 2018, at 12:06 PM, Thomas Weise <th...@apache.org>
>>>>>>> wrote:
>>>>>>> > >>>
>>>>>>> > >>> I like Till's proposal to notify the participants on the PR to
>>>>>>> PTAL.
>>>>>>> But
>>>>>>> > >> I
>>>>>>> > >>> would also suggest to auto-close when no action is taken, with
>>>>>>> a
>>>>>>> friendly
>>>>>>> > >>> reminder that PRs can be reopened anytime.
>>>>>>> > >>>
>>>>>>> > >>> The current situation with 350 open PRs may send a signal to
>>>>>>> contributors
>>>>>>> > >>> that it may actually be too much hassle to get a change
>>>>>>> committed in
>>>>>>> > >> Flink.
>>>>>>> > >>> Since the count keeps on rising, this is also not a past
>>>>>>> problem.
>>>>>>> Pruning
>>>>>>> > >>> inactive PRs may help, that will also give a more accurate
>>>>>>> picture of
>>>>>>> the
>>>>>>> > >>> lack of review bandwidth, if that is the root cause.
>>>>>>> > >>>
>>>>>>> > >>> Thanks,
>>>>>>> > >>> Thomas
>>>>>>> > >>>
>>>>>>> > >>>
>>>>>>> > >>>
>>>>>>> > >>>
>>>>>>> > >>>
>>>>>>> > >>> On Tue, May 15, 2018 at 8:24 AM, Ted Yu <yu...@gmail.com>
>>>>>>> wrote:
>>>>>>> > >>>
>>>>>>> > >>>> How does the bot decide whether the PR is waiting for reviews
>>>>>>> or is
>>>>>>> > >> being
>>>>>>> > >>>> abandoned by contributor ?
>>>>>>> > >>>>
>>>>>>> > >>>> How about letting the bot count the number of times
>>>>>>> contributor pings
>>>>>>> > >>>> committer(s) for review ?
>>>>>>> > >>>> When unanswered ping count crosses some threshold, say 3, the
>>>>>>> bot
>>>>>>> > >> publishes
>>>>>>> > >>>> the JIRA and PR somewhere.
>>>>>>> > >>>>
>>>>>>> > >>>> Cheers
>>>>>>> > >>>>
>>>>>>> > >>>> On Tue, May 15, 2018 at 8:19 AM, Till Rohrmann <
>>>>>>> trohrmann@apache.org>
>>>>>>> > >>>> wrote:
>>>>>>> > >>>>
>>>>>>> > >>>>> I'm a bit torn here because I can see the pros and cons for
>>>>>>> both
>>>>>>> sides.
>>>>>>> > >>>>>
>>>>>>> > >>>>> Maybe a compromise could be to not have a closing but a
>>>>>>> monitoring
>>>>>>> bot
>>>>>>> > >>>>> which notifies us about inactive PRs. This could then
>>>>>>> trigger an
>>>>>>> > >>>>> investigation of the underlying problem and ultimately lead
>>>>>>> to a
>>>>>>> > >>>> conscious
>>>>>>> > >>>>> decision to close or keep the PR open. As such it is not
>>>>>>> strictly
>>>>>>> > >>>> necessary
>>>>>>> > >>>>> to have such a bot but it would at least remind us to make a
>>>>>>> decision
>>>>>>> > >>>> about
>>>>>>> > >>>>> older PRs with no activity.
>>>>>>> > >>>>>
>>>>>>> > >>>>> Cheers,
>>>>>>> > >>>>> Till
>>>>>>> > >>>>>
>>>>>>> > >>>>> On Tue, May 15, 2018 at 3:26 PM, Chesnay Schepler <
>>>>>>> chesnay@apache.org>
>>>>>>> > >>>>> wrote:
>>>>>>> > >>>>>
>>>>>>> > >>>>>> /So far I did it twice for older PRs. In both cases I
>>>>>>> didn’t get
>>>>>>> any
>>>>>>> > >>>>>> response and I even forgot in which PRs I had asked this
>>>>>>> question,
>>>>>>> so
>>>>>>> > >>>>> now I
>>>>>>> > >>>>>> can not even close them :S/
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> To be honest this sounds more like an issue with how your
>>>>>>> organize
>>>>>>> > >> your
>>>>>>> > >>>>>> work. No amount of closing PRs can fix that.
>>>>>>> > >>>>>> With GitBox we can assign reviewers to a PR, but I'm not
>>>>>>> sure
>>>>>>> whether
>>>>>>> > >>>> it
>>>>>>> > >>>>>> only allows committers to assign people.
>>>>>>> > >>>>>> Bookmarks or text files might help as well./
>>>>>>> > >>>>>> /
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> /Regarding only 30% blocked on contributor. I wonder what
>>>>>>> would be
>>>>>>> > >> this
>>>>>>> > >>>>>> number if we tried to ask in the rest of old PRs “Hey, are
>>>>>>> you
>>>>>>> still
>>>>>>> > >>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>> waiting for a
>>>>>>> > >>>>>> reviewer for X months, it doesn’t mean that’s not
>>>>>>> abandoned. Even
>>>>>>> if
>>>>>>> > >> it
>>>>>>> > >>>>>> doesn’t, reducing our overhead by those 30% of the PRs is
>>>>>>> something./
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> No doubt the number would be higher if we were to go back,
>>>>>>> but as i
>>>>>>> > >>>>>> explained earlier that is not a reason to close it. If a PR
>>>>>>> is
>>>>>>> > >>>> abandoned
>>>>>>> > >>>>>> because we messed up we should still /try /to get it in.
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> /This is kind of whole point of what I was proposing. If
>>>>>>> the PR
>>>>>>> author
>>>>>>> > >>>> is
>>>>>>> > >>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>> timeout,
>>>>>>> > >> that’s
>>>>>>> > >>>>>> great. Gives us even more sense of urgency to review it./
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> Unfortunately knowing that it is more urgent is irrelevant,
>>>>>>> as we
>>>>>>> > >>>>>> currently don't have the manpower to review them. Reviving
>>>>>>> them now
>>>>>>> > >>>> would
>>>>>>> > >>>>>> serve no purpose. The alternative is to wait until more
>>>>>>> people
>>>>>>> become
>>>>>>> > >>>>>> active reviewers.
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> /As a last resort, closing PR after timeout is not the end
>>>>>>> of the
>>>>>>> > >>>> world.
>>>>>>> > >>>>>> It always can be reopened./
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> Let's be realistic here, it will not be reopened.
>>>>>>> > >>>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>> On 15.05.2018 14:21, Piotr Nowojski wrote:
>>>>>>> > >>>>>>
>>>>>>> > >>>>>>> I agree that we have other, even more important, problems
>>>>>>> with
>>>>>>> > >>>> reviewing
>>>>>>> > >>>>>>> PR and community, but that shouldn’t block us from trying
>>>>>>> to
>>>>>>> clean up
>>>>>>> > >>>>>>> things a little bit and minimise the effort needed for
>>>>>>> reviewing
>>>>>>> PRs.
>>>>>>> > >>>>> Now
>>>>>>> > >>>>>>> before reviewing/picking older PRs I had to ask this “Hey,
>>>>>>> are you
>>>>>>> > >>>> still
>>>>>>> > >>>>>>> interested in merging this?” manually and wait for the
>>>>>>> response.
>>>>>>> If
>>>>>>> > >> it
>>>>>>> > >>>>>>> doesn’t come, I have to remember to go back and close PR,
>>>>>>> which I
>>>>>>> of
>>>>>>> > >>>>> course
>>>>>>> > >>>>>>> forget to do. Bah, so far I did it twice for older PRs. In
>>>>>>> both
>>>>>>> cases
>>>>>>> > >>>> I
>>>>>>> > >>>>>>> didn’t get any response and I even forgot in which PRs I
>>>>>>> had asked
>>>>>>> > >>>> this
>>>>>>> > >>>>>>> question, so now I can not even close them :S Wasted
>>>>>>> effort and
>>>>>>> > >> wasted
>>>>>>> > >>>>> time
>>>>>>> > >>>>>>> on context switching for me and for everyone else that
>>>>>>> will have
>>>>>>> to
>>>>>>> > >>>>> scroll
>>>>>>> > >>>>>>> pass or look on those PR to check their status.
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>> Keep in mind that I am not proposing to close the PR
>>>>>>> automatically
>>>>>>> > >>>>>>> straight on after 3 months of inactivity. Only after
>>>>>>> asking a
>>>>>>> > >> question
>>>>>>> > >>>>>>> whether original contributor is still there and he is
>>>>>>> interested
>>>>>>> in
>>>>>>> > >>>> the
>>>>>>> > >>>>> PR
>>>>>>> > >>>>>>> to be reviewed.
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>> for Flink 1.5, I merged a contribution from PR #1990 after
>>>>>>> it was
>>>>>>> > >>>>>>>> requested a few times by users.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>> This is kind of whole point of what I was proposing. If
>>>>>>> the PR
>>>>>>> author
>>>>>>> > >>>> is
>>>>>>> > >>>>>>> still there, and can respond/bump/interrupt the closing
>>>>>>> timeout,
>>>>>>> > >>>> that’s
>>>>>>> > >>>>>>> great. Gives us even more sense of urgency to review it.
>>>>>>> On the
>>>>>>> other
>>>>>>> > >>>>> hand
>>>>>>> > >>>>>>> if there is no response (no interest from the author for
>>>>>>> whatever
>>>>>>> the
>>>>>>> > >>>>>>> reasons) and nobody so far has picked this PR to
>>>>>>> review/merge,
>>>>>>> what’s
>>>>>>> > >>>>> the
>>>>>>> > >>>>>>> point of keeping such PR open? As a last resort, closing
>>>>>>> PR after
>>>>>>> > >>>>> timeout
>>>>>>> > >>>>>>> is not the end of the world. It always can be reopened.
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>> Regarding only 30% blocked on contributor. I wonder what
>>>>>>> would be
>>>>>>> > >> this
>>>>>>> > >>>>>>> number if we tried to ask in the rest of old PRs “Hey, are
>>>>>>> you
>>>>>>> still
>>>>>>> > >>>>>>> interested in reviewing/merging this?”. If old PR is
>>>>>>> waiting for a
>>>>>>> > >>>>> reviewer
>>>>>>> > >>>>>>> for X months, it doesn’t mean that’s not abandoned. Even
>>>>>>> if it
>>>>>>> > >>>> doesn’t,
>>>>>>> > >>>>>>> reducing our overhead by those 30% of the PRs is something.
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>> Piotrek
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>> On 15 May 2018, at 10:10, Fabian Hueske <fh...@gmail.com>
>>>>>>> wrote:
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> I'm with Chesnay on this issue.
>>>>>>> > >>>>>>>> Stale PRs, i.e., a PR where a contributor becomes
>>>>>>> inactive, are
>>>>>>> one
>>>>>>> > >>>> of
>>>>>>> > >>>>>>>> our
>>>>>>> > >>>>>>>> smallest issues, IMO.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> There are more reasons for the high number of PRs.
>>>>>>> > >>>>>>>> * Lack of timely reviews.
>>>>>>> > >>>>>>>> * Not eagerly closing PRs that have no or very little
>>>>>>> chance of
>>>>>>> > >> being
>>>>>>> > >>>>>>>> merged. Common reasons are
>>>>>>> > >>>>>>>> 1) lack of interest in the feature by committers,
>>>>>>> > >>>>>>>> 2) too extensive changes and hence time consuming
>>>>>>> reviews, or
>>>>>>> > >>>>>>>> 3) bad quality.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> For 1), there are lots of older JIRA issues, that have low
>>>>>>> priority
>>>>>>> > >>>> but
>>>>>>> > >>>>>>>> which are picked up by contributors. In the contribution
>>>>>>> guidelines
>>>>>>> > >>>> we
>>>>>>> > >>>>>>>> ask
>>>>>>> > >>>>>>>> contributors to let us know when they want to work on an
>>>>>>> issue.
>>>>>>> So
>>>>>>> > >>>> far
>>>>>>> > >>>>>>>> our
>>>>>>> > >>>>>>>> attitude has been, yes sure go ahead. I've seen very
>>>>>>> little
>>>>>>> attempts
>>>>>>> > >>>> of
>>>>>>> > >>>>>>>> warning somebody to work on issues that won't be easily
>>>>>>> merged.
>>>>>>> > >>>>>>>> Once a PR has been opened, we should also be honest and
>>>>>>> let
>>>>>>> > >>>>> contributors
>>>>>>> > >>>>>>>> know that it has no chance or might take a while to get
>>>>>>> reviewed.
>>>>>>> > >>>>>>>> For 2) this is typically not so much of an issue if the
>>>>>>> feature
>>>>>>> is
>>>>>>> > >>>>>>>> interesting. However, if 1) and 2) meet, chances to get a
>>>>>>> change
>>>>>>> in
>>>>>>> > >>>>> drop
>>>>>>> > >>>>>>>> even more.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> A common "strategy" to deal with PRs that fall into 1),
>>>>>>> 2), or
>>>>>>> 3) is
>>>>>>> > >>>> to
>>>>>>> > >>>>>>>> not
>>>>>>> > >>>>>>>> look at them or giving a shallow review.
>>>>>>> > >>>>>>>> Of course, contributors become unresponsive if we don't
>>>>>>> look at
>>>>>>> > >> their
>>>>>>> > >>>>> PRs
>>>>>>> > >>>>>>>> for weeks or months. But that's not their fault.
>>>>>>> > >>>>>>>> Instead, I think we should be honest and communicate the
>>>>>>> chances
>>>>>>> of
>>>>>>> > >> a
>>>>>>> > >>>>> PR
>>>>>>> > >>>>>>>> more clearly.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> Browsing over the list of open PRs, I feel that most
>>>>>>> older PRs
>>>>>>> fall
>>>>>>> > >>>>> into
>>>>>>> > >>>>>>>> the category of low-priority (or even unwanted) features.
>>>>>>> > >>>>>>>> Bug fixes or features that the committers care about
>>>>>>> usually
>>>>>>> make it
>>>>>>> > >>>>> into
>>>>>>> > >>>>>>>> the code base.
>>>>>>> > >>>>>>>> In case a contributor becomes inactive, committers often
>>>>>>> take
>>>>>>> over
>>>>>>> > >> an
>>>>>>> > >>>>>>>> push
>>>>>>> > >>>>>>>> a contribution over the line.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> So, I do not support an auto-close mechanism.
>>>>>>> > >>>>>>>> I think a better way to address the issue is better
>>>>>>> communication,
>>>>>>> > >>>> more
>>>>>>> > >>>>>>>> eagerly closing PRs with no chance, and putting more
>>>>>>> reviewing
>>>>>>> > >>>> effort.
>>>>>>> > >>>>>>>> IMO, we should only eagerly close PRs that have no chance
>>>>>>> of
>>>>>>> being
>>>>>>> > >>>>>>>> merged.
>>>>>>> > >>>>>>>> PRs with low-prio features might be picked up later (for
>>>>>>> Flink
>>>>>>> 1.5,
>>>>>>> > >> I
>>>>>>> > >>>>>>>> merged a contribution from PR #1990 after it was
>>>>>>> requested a few
>>>>>>> > >>>> times
>>>>>>> > >>>>> by
>>>>>>> > >>>>>>>> users).
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> However, I think we could do a pass over the oldest PRs
>>>>>>> and
>>>>>>> check if
>>>>>>> > >>>> we
>>>>>>> > >>>>>>>> can
>>>>>>> > >>>>>>>> close a bunch.
>>>>>>> > >>>>>>>> There are quite a few contributions (many for flink-ml)
>>>>>>> that I
>>>>>>> don't
>>>>>>> > >>>>> see
>>>>>>> > >>>>>>>> a
>>>>>>> > >>>>>>>> chance for getting merged.
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> Best, Fabian
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> -
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> 2018-05-15 9:13 GMT+02:00 Chesnay Schepler <
>>>>>>> chesnay@apache.org>:
>>>>>>> > >>>>>>>>
>>>>>>> > >>>>>>>> -1
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> For clarification (since the original mail indicates
>>>>>>> otherwise),
>>>>>>> > >> the
>>>>>>> > >>>>>>>>> number of pull requests that this would affect is fairly
>>>>>>> small.
>>>>>>> > >>>>>>>>> Only about 25-30% of all open PRs are blocked on the
>>>>>>> contributor,
>>>>>>> > >>>> the
>>>>>>> > >>>>>>>>> remaining ones are actually blocked on the review.
>>>>>>> > >>>>>>>>> Thus is reject the premise that one has to search
>>>>>>> through that
>>>>>>> many
>>>>>>> > >>>>> PRs
>>>>>>> > >>>>>>>>> to
>>>>>>> > >>>>>>>>> find something to review, there is plenty.
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> I believe it to be highly unfair for us to close PRs due
>>>>>>> to
>>>>>>> > >>>>> inactivity,
>>>>>>> > >>>>>>>>> when the primary cause has been /our /own inactivity.
>>>>>>> > >>>>>>>>> If a PR is opened and the first comment comes in 3
>>>>>>> months late,
>>>>>>> > >>>> then I
>>>>>>> > >>>>>>>>> don't blame the contributor for not responding.
>>>>>>> > >>>>>>>>> IMO we owe it to the contributor to evaluate their PR,
>>>>>>> and if
>>>>>>> > >>>>> necessary
>>>>>>> > >>>>>>>>> bring it to a merge-able state (to a certain extend).
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> There's also the fact that closing these PRs outright
>>>>>>> would
>>>>>>> waste a
>>>>>>> > >>>>> lot
>>>>>>> > >>>>>>>>> of
>>>>>>> > >>>>>>>>> good contributions.
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> Finally, this solution is overkill for what we want to
>>>>>>> achieve.
>>>>>>> If
>>>>>>> > >>>> we
>>>>>>> > >>>>>>>>> want
>>>>>>> > >>>>>>>>> to make it easier to find PRs to review all we need is
>>>>>>> > >>>>>>>>> GitBox integration and tagging or PRs. That's it. We
>>>>>>> could have
>>>>>>> a
>>>>>>> > >>>>> /fully
>>>>>>> > >>>>>>>>> /tagged PR list /tomorrow/, if we really wanted to.
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> On 15.05.2018 05:10, Ted Yu wrote:
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>>> bq. this pull request requires a review, please simply
>>>>>>> write any
>>>>>>> > >>>>>>>>>> comment.
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>> Shouldn't the wording of such comment be known before
>>>>>>> hand ?
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>> Otherwise pull request waiting for committers' review
>>>>>>> may be
>>>>>>> > >>>>>>>>>> mis-classified.
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>> Cheers
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>> On Mon, May 14, 2018 at 7:59 PM, blues zheng <
>>>>>>> kisimple@163.com>
>>>>>>> > >>>>> wrote:
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>> +1 for the proposal.
>>>>>>> > >>>>>>>>>>
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> Best,
>>>>>>> > >>>>>>>>>>> blues
>>>>>>> > >>>>>>>>>>> On 05/14/2018 20:58, Ufuk Celebi wrote:
>>>>>>> > >>>>>>>>>>> Hey Piotr,
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> thanks for bringing this up. I really like this
>>>>>>> proposal and
>>>>>>> also
>>>>>>> > >>>>> saw
>>>>>>> > >>>>>>>>>>> it work successfully at other projects. So +1 from my
>>>>>>> side.
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> - I like the approach with a notification one week
>>>>>>> before
>>>>>>> > >>>>>>>>>>> automatically closing the PR
>>>>>>> > >>>>>>>>>>> - I think a bot will the best option as these kinds of
>>>>>>> things
>>>>>>> are
>>>>>>> > >>>>>>>>>>> usually followed enthusiastically in the beginning but
>>>>>>> eventually
>>>>>>> > >>>>>>>>>>> loose traction
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> We can enable better integration with GitHub by using
>>>>>>> ASF
>>>>>>> GitBox
>>>>>>> > >>>>>>>>>>> (https://gitbox.apache.org/setup/) but we should
>>>>>>> discuss that
>>>>>>> in
>>>>>>> > >>>> a
>>>>>>> > >>>>>>>>>>> separate thread.
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> – Ufuk
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> On Mon, May 14, 2018 at 12:04 PM, Piotr Nowojski
>>>>>>> > >>>>>>>>>>> <pi...@data-artisans.com> wrote:
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> Hey,
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> We have lots of open pull requests and quite some of
>>>>>>> them are
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> stale/abandoned/inactive. Often such old PRs are
>>>>>>> impossible
>>>>>>> to
>>>>>>> > >>>>> merge
>>>>>>> > >>>>>>>>>>> due
>>>>>>> > >>>>>>>>>>> to
>>>>>>> > >>>>>>>>>>> conflicts and it’s easier to just abandon and rewrite
>>>>>>> them.
>>>>>>> > >>>>> Especially
>>>>>>> > >>>>>>>>>>> there are some PRs which original contributor created
>>>>>>> long
>>>>>>> time
>>>>>>> > >>>> ago,
>>>>>>> > >>>>>>>>>>> someone else wrote some comments/review and… that’s
>>>>>>> about it.
>>>>>>> > >>>>> Original
>>>>>>> > >>>>>>>>>>> contributor never shown up again to respond to the
>>>>>>> comments.
>>>>>>> > >>>>>>>>>>> Regardless
>>>>>>> > >>>>>>>>>>> of
>>>>>>> > >>>>>>>>>>> the reason such PRs are clogging the GitHub, making it
>>>>>>> difficult
>>>>>>> > >>>> to
>>>>>>> > >>>>>>>>>>> keep
>>>>>>> > >>>>>>>>>>> track of things and making it almost impossible to
>>>>>>> find a
>>>>>>> little
>>>>>>> > >>>> bit
>>>>>>> > >>>>>>>>>>> old
>>>>>>> > >>>>>>>>>>> (for example 3+ months) PRs that are still valid and
>>>>>>> waiting
>>>>>>> for
>>>>>>> > >>>>>>>>>>> reviews.
>>>>>>> > >>>>>>>>>>> To do something like that, one would have to dig
>>>>>>> through tens
>>>>>>> or
>>>>>>> > >>>>>>>>>>> hundreds
>>>>>>> > >>>>>>>>>>> of abandoned PRs.
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> What I would like to propose is to agree on some
>>>>>>> inactivity
>>>>>>> dead
>>>>>>> > >>>>> line,
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> lets say 3 months. After crossing such deadline, PRs
>>>>>>> should
>>>>>>> be
>>>>>>> > >>>>>>>>>>> marked/commented as “stale”, with information like:
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> “This pull request has been marked as stale due to 3
>>>>>>> months of
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> inactivity. It will be closed in 1 week if no further
>>>>>>> activity
>>>>>>> > >>>>>>>>>>> occurs. If
>>>>>>> > >>>>>>>>>>> you think that’s incorrect or this pull request
>>>>>>> requires a
>>>>>>> > >> review,
>>>>>>> > >>>>>>>>>>> please
>>>>>>> > >>>>>>>>>>> simply write any comment.”
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> Either we could just agree on such policy and enforce
>>>>>>> it
>>>>>>> manually
>>>>>>> > >>>>>>>>>>>> (maybe
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> with some simple tooling, like a simple script to list
>>>>>>> inactive
>>>>>>> > >>>>> PRs -
>>>>>>> > >>>>>>>>>>> seems
>>>>>>> > >>>>>>>>>>> like couple of lines in python by using PyGithub) or
>>>>>>> we could
>>>>>>> > >>>> think
>>>>>>> > >>>>>>>>>>> about
>>>>>>> > >>>>>>>>>>> automating this action. There are some bots that do
>>>>>>> exactly
>>>>>>> this
>>>>>>> > >>>>> (like
>>>>>>> > >>>>>>>>>>> this
>>>>>>> > >>>>>>>>>>> one: https://github.com/probot/stale <
>>>>>>> https://github.com/probot/
>>>>>>> > >>>>> stale
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> ),
>>>>>>> > >>>>>>>>>>> but probably they would need to be adopted to
>>>>>>> limitations of
>>>>>>> our
>>>>>>> > >>>>>>>>>>> Apache
>>>>>>> > >>>>>>>>>>> repository (we can not add labels and we can not close
>>>>>>> the PRs
>>>>>>> > >> via
>>>>>>> > >>>>>>>>>>> GitHub).
>>>>>>> > >>>>>>>>>>>
>>>>>>> > >>>>>>>>>>> What do you think about it?
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>> Piotrek
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>>>>
>>>>>>> > >>>>>>>>>
>>>>>>> > >>>>>>>
>>>>>>> > >>>>>>
>>>>>>> > >>>>>
>>>>>>> > >>>>
>>>>>>> > >>
>>>>>>> > >>
>>>>>>>
>>>>>>