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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>> 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 < >>>>> [email protected]> 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 <[email protected]> >>>>>> 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 <[email protected]> >>>>>>> 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 < >>>>>>>> [email protected]> >>>>>>>> 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 <[email protected]> >>>>>>>> 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 <[email protected]>: >>>>>>>> > > >>>>>>>> > >> 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 <[email protected]> >>>>>>>> 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 <[email protected]> >>>>>>>> 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 < >>>>>>>> [email protected]> >>>>>>>> > >>>> 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 < >>>>>>>> [email protected]> >>>>>>>> > >>>>> 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 < >>>>>>>> [email protected]> 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 < >>>>>>>> [email protected]>: >>>>>>>> > >>>>>>>> >>>>>>>> > >>>>>>>> -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 < >>>>>>>> [email protected]> >>>>>>>> > >>>>> 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 >>>>>>>> > >>>>>>>>>>> <[email protected]> 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 >>>>>>>> > >>>>>>>>>>>> >>>>>>>> > >>>>>>>>>>>> >>>>>>>> > >>>>>>>>> >>>>>>>> > >>>>>>> >>>>>>>> > >>>>>> >>>>>>>> > >>>>> >>>>>>>> > >>>> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> >>>>>>>
