Otto, I think using the embedded option offered by Github is better as I don't think submitting requests to Apache Infra for this is great.
What would be the best way forward on this subject? Andy, do you still have concerns and/or comments based on the feedback? Should it go through a formal VOTE thread? Happy to take care of it if there is a consensus. Thanks, Pierre Le dim. 16 sept. 2018 à 15:38, Otto Fowler <ottobackwa...@gmail.com> a écrit : > In Metron we just put something like this in place, but not using a bot. > We limit it to PR’s where the contributor has gone inactive. > https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines > > 2.6.1 Inactive Pull Requests > > Contributions can often take a significant amount of time to complete the > code review process. This process requires active participation from the > contributor. If the contributor is unable to actively participate, the pull > request is unlikely to successfully complete this process. > > Pull Requests that have failed to receive active participation from the > contributor for an extended period of time risk being abandoned. Any > committer can submit a request for Apache Infra to close a pull request > that has been abandoned according to the following guidelines. > > - A pull request is 'inactive' if no comments or updates have been made > by the contributor in the previous 4 weeks. > > > - For any 'inactive' pull request, a committer can request from the > contributor justification for keeping the pull request open. > > > - The committer's request should be made as a public comment on the pull > request. The committer should refer the contributor to these development > guidelines for inactive pull requests. > > > - If the contributor publically responds to the request, the pull > request is no longer consider 'inactive'. > > > - If the contributor does not respond to the request within 2 weeks, the > pull request is considered 'abandoned'. > > > - A committer can cast a -1 vote on any 'abandoned' pull request using > these development guidelines as justification. > > > - A committer can submit a request to Apache Infra to close the > 'abandoned' pull request based on this -1 vote. > > > > On September 15, 2018 at 22:22:17, Mike Thomsen (mikerthom...@gmail.com) > wrote: > > I am in favor of these changes. One thing we should consider is looking for > old PRs that are close enough to being done that we could rebase them, > clean them up a little and add them to master. Just a thought. > > Thanks, > > Mike > > On Sat, Sep 15, 2018 at 1:34 PM Mark Payne <marka...@hotmail.com> wrote: > > > I'm 100% on-board here. I brought up this same topic a couple of months > > ago, > > but the thread kind of digressed (as these things tend to do on large > > mailing lists). > > I am in favor of a 30 day period with a reminder that gives the > > contributor an extra > > week before closing the PR. If the contributor is simply busy and not > able > > to finish up > > for a while, a simple comment to that effect would allow the PR to stay > > open. At least > > this way we know whether a PR is in-progress or just lingering and will > > never get > > any progress. > > > > While there are times that the committers are at fault, I think that's a > > separate discussion > > that we can have. Both sides of the equation have to be addressed, and > > that should not > > prevent us from closing out stale PR's. > > > > That being said, I think closing out stale PR's will actually improve the > > committers' review > > rate. I sometimes start looking through the list of PR's to review and > > then get overwhelmed > > because there are so many of them right now, and almost all of them have > a > > comment of > > some sort on them. Often I have no idea if the PR is still being worked > on > > or not. If we reduce > > the number of open PR's down to only those that are still being worked, > > it's a lot less overwhelming > > for the committers to look through and see what needs to be reviewed. > > > > It's also worth nothing that this is not just something that Beam does. I > > know Kubernetes has a similar > > mechanism in place, and I'm guessing that this is a pretty common > practice > > in general. And one that > > I think will definitely help out both committers and contributors and > make > > the project more approachable > > from those who are interested in getting involved. > > > > Thanks > > -Mark > > > > > > > On Sep 15, 2018, at 12:08 PM, Jeremy Dyer <jdy...@gmail.com> wrote: > > > > > > Andy - That’s a good point. What I had in my mind was sort of like this > > .... > > > > > > - After 30 days alert is sent to author if no activity/comment not just > > commits.They would still have something like a week > > > - If code is complicated they don’t have to finish it just simply > > comment on PR to stop it from being closed > > > - I like this because when they comment they can say things like. > “Sorry > > this is taking longer because of problem XYZ I’m having” others in the > > community can see this and offer input so it helps on collaboration > > > - This also helps people watching the PRs and interested in using them > > have a much more clear and accurate understanding of where the PR > actually > > stands progress wise so they can more accurately determine when it will > be > > available to them > > > - To your last point, which is a good one, they would simply comment > > with a gentle reminder that they would like for someone to review. > > > > > > Thoughts? I’m sure I’m missing something there but that’s sort of how I > > imagine it working > > > > > > - Jeremy Dyer > > > > > > Thanks - Jeremy Dyer > > > > > > ________________________________ > > > From: Andy LoPresto <alopresto.apa...@gmail.com> > > > Sent: Saturday, September 15, 2018 11:57 AM > > > To: dev@nifi.apache.org > > > Subject: Re: [DISCUSS] Stale PRs > > > > > > Jeremy, > > > > > > What about in the scenario where the submitter does everything and we > > (the committers) are slow? I’m not saying we shouldn’t try to fix all the > > problems, just that I see a lot more of the latter happening. > > > > > > Andy LoPresto > > > alopre...@apache.org > > > alopresto.apa...@gmail.com > > > PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 > > > > > >> On Sep 15, 2018, at 08:51, Pierre Villard < > pierre.villard...@gmail.com> > > > wrote: > > >> > > >> Andy, > > >> > > >> Totally get your points. I imagine that introducing this approach > would > > >> help keeping dynamic exchanges on pull requests. > > >> > > >> In case a PR needs complex/time consuming work (or in case the author > is > > >> just not in a position to process comments), I think we could have two > > >> approaches: > > >> - the PR is considered stale after 60 days but is actually closed one > > week > > >> later. I think it leaves time for someone (ideally the author) to > > comment > > >> and give an update so that the PR is not considered stale anymore, no? > > >> - for important PRs, it's possible to "remove" this mechanism using > > >> specific labels but I guess we would have to ask ASF infra if we want > to > > >> have rights to add labels on PRs (?) > > >> > > >> Pierre > > >> > > >> > > >> > > >> > > >> Le sam. 15 sept. 2018 à 17:44, Andy LoPresto < > > alopresto.apa...@gmail.com> a > > >> écrit : > > >> > > >>> Pierre, > > >>> > > >>> I’m going to delay my response on that proposal while I ask for (aka > > >>> should gather on my own) some information. Is that really our > problem? > > By > > >>> that, I mean are stale PRs where we are getting bogged down? I am > sure > > >>> there are some old ones that should be closed out. My larger concern > is > > >>> that even new PRs don’t get reviewed immediately for a number of > > reasons. > > >>> > > >>> 1. Balance of committers to submissions. As the project continues to > > grow, > > >>> we have far more people providing code than can review it. > > >>> 2. Quality of PR. Not that the code is necessarily bad, but the PR > > doesn’t > > >>> clearly explain the problem and how they are solving it, provide test > > >>> cases, provide templates or a Docker container if interacting with an > > >>> external service, etc. All of these things add up to make the cost of > > >>> reviewing higher. > > >>> 3. What PRs cover. Previously, there was still a lot of low-hanging > > fruit, > > >>> and less complexity. While the project is still fairly cleanly > > organized, > > >>> many PRs now are less “add this simple functionality” and more > “improve > > >>> this complicated feature.” > > >>> > > >>> I guess I would not have a problem with your proposal, but I do > wonder > > if > > >>> there are more effective ways to reduce the backlog by identifying > > other > > >>> areas of improvement. > > >>> > > >>> Andy LoPresto > > >>> alopre...@apache.org > > >>> alopresto.apa...@gmail.com > > >>> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 > > >>> > > >>>> On Sep 15, 2018, at 08:33, Pierre Villard < > > pierre.villard...@gmail.com> > > >>> wrote: > > >>>> > > >>>> Hi, > > >>>> > > >>>> The number of open PRs is still growing and it could make think > people > > >>> that > > >>>> the project is not healthy/active (even though a quick look at the > > last > > >>>> commit date or the commits rate is a clear indication that the > > project is > > >>>> healthy). > > >>>> > > >>>> In order to encourage people to review code and keep active > > discussions > > >>> on > > >>>> the PRs, I suggest to find a way to keep this number as small as > > >>> possible. > > >>>> To do so, I'd like to ask the wider community if the approach taken > > by a > > >>>> project like Apache Beam would be a good idea: > > >>>> > > >>>> "A pull request becomes stale after its author fails to respond to > > >>>> actionable comments for 60 days. Author of a closed pull request is > > >>> welcome > > >>>> to reopen the same pull request again in the future." > > >>>> > > >>>> This approach is managed by a file [1] in the .github directory of > the > > >>>> repository. > > >>>> > > >>>> What do you think about this approach? > > >>>> > > >>>> [1] https://github.com/apache/beam/blob/master/.github/stale.yml > > >>>> > > >>>> Pierre > > >>> > > > > >
