Re: [DISCUSS] Stale PRs
t'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 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 > > > 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 reques
Re: [DISCUSS] Stale PRs
l 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 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 > 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 expla
Re: [DISCUSS] Stale PRs
gt; > > 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 > > > 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 functi
Re: [DISCUSS] Stale PRs
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 > > 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 > 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 > >>> > >
Re: [DISCUSS] Stale PRs
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 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 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 > > 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 > 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 AS
Re: [DISCUSS] Stale PRs
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 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 > 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 >> 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 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. >>&g
Re: [DISCUSS] Stale PRs
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 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 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 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 >> 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 >>
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 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 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 >> 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 >>
Re: [DISCUSS] Stale PRs
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 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 > 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 >
Re: [DISCUSS] Stale PRs
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 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
[DISCUSS] Stale PRs
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
