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 > >>> > >
