You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Michael Marshall <mm...@apache.org> on 2021/12/03 16:56:52 UTC
[DISCUSS] How to handle stale PRs
Hi Pulsar Community,
I am excited to start contributing as a committer! I have a question
about our process for closing stale PRs.
We have ~300 open PRs right now. Do we have any guidelines on closing
stale PRs? Of course we don't want to ignore important bug fixes, but
we also don't want to clutter our repo with open PRs that won't get merged.
For example, I reviewed this PR [0] about 3 months ago. The
contributor has not yet responded to my feedback and it doesn't seem
to fix an actual bug, so I think it is a candidate for closure. Here
is another example [1]. I closed this one because it had merge
conflicts with a commit that fixed the same underlying issue.
Note that our committer guidelines [2] do not provide guidance on this subject.
[0] - https://github.com/apache/pulsar/pull/11237
[1] - https://github.com/apache/pulsar/pull/11162
[2] - https://github.com/apache/pulsar/wiki/Committer-Guide
Thanks,
Michael
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
I just saw another project - https://github.com/openmessaging/benchmark uses probot-stale https://github.com/probot/stale
This looks like it has all the features needed to close both stale issues and PRs. It allows labels to be used to prevent closure of certain issues and PRs.
Here is their configuration: https://github.com/openmessaging/benchmark/blob/master/.github/stale.yml
This bot is allowed in GitHub.com/apache/ where 11 repositories are currently using it. When we are ready we will simply create an INFRA JIRA.
> On Dec 15, 2021, at 4:15 PM, Dave Fisher <wa...@apache.org> wrote:
>
>
>
>> On Dec 15, 2021, at 4:06 PM, Matteo Merli <mm...@apache.org> wrote:
>>
>>> Is #3267 Support set publish time on broker side one of those very valuable ideas that was later rejected, likely for performance reasons?
>>
>> No, this was one that was superseded by other changes.
>
> Then I’ll close it.
>
>>
>>> One problem with the current state is that PRs and even higher level ideas
>>> have a shelf life. Declaring PR bankruptcy does in fact solve this problem.
>>
>> I don't believe that is true in all cases and I absolutely don't
>> believe that it is not possible to keep up with the PRs, when the
>> reviewing workload is well balanced.
>
>
>>
>> I'm seeing a lot of opinions here, but at the end of the day the
>> people doing the hard work of reviewing are always the same few ones.
>
> (1) These are opinions about how to do the work. If you want someone to JFDI it then I’m happy to start closing and labeling as I suggested.
I started closing PRs with a new label - status/stale
https://github.com/apache/pulsar/issues?q=label%3Astatus%2Fstale+is%3Aclosed
>
> (2) There is a kind of deference being shown to those individuals based on who the contributor selects for review. I wish there was a way for a contributor to ask the dev list for a review.
I plan to research how we might modify how reviews are requested. I think that can be in another thread.
>
>
>>
>>> Once we have guidance, I am happy to add it to the Committer Guide on
>> the wiki [0].
>>
>> Michael, I agree 100% with that. We should write clear guidelines to
>> describe when it makes sense to close, leave for the record, call for
>> "help" to continue working on and so on. That will help committers and
>> contributors.
>>
>>> Matteo, your comment raises an additional question for me. What are
>>> Apache's rules for completing someone else's contribution? If someone
>>> opens a PR to fix a bug, but it is incomplete and they become
>>> unresponsive, how can we move their contribution forward? These are
>>> the PRs we don't want to close.
>>
>> I don't think there is any problem in completing someone else's PR,
>> provided that:
>> * The original author is non-responsive or has no time to work on it
>> at the moment (otherwise it would be kind of rude).
>> * We give the right credit to the original author (github has good
>> support for multiple authorship of a commit)
>>
>> Continuing with a PR is not very different from merging the WIP and
>> fixing it later in a second commit, from a legal perspective.
>>
>> IANAL, though *AFAIU*, when a contributor is opening a PR is already
>> assigning the IP to the ASF. A committer will merge that code (after
>> due diligence that it doesn't contain inadmissible code), but the code
>> is already "donated to the ASF" at the moment of the PR.
>
> +1.
>
> Regards,
> Dave
>
>>
>>
>> --
>> Matteo Merli
>> <mm...@apache.org>
>>
>>
>> On Wed, Dec 15, 2021 at 3:14 PM Chris Herzog <ch...@tibco.com.invalid> wrote:
>>>
>>> It isn't even an issue related to OSS - every long lived project suffers
>>> from this same issue. Whether it's a long lingering defect report or a fix
>>> that never got integrated in a timely manner, time wounds all heels.
>>>
>>> Careful considered review is perfection which can't be hit; if it could be
>>> done, the situation would never have occured in the first place. Having a
>>> time-to-live is pragmatic, not perfect, but pragmatic.
>>>
>>> As Jonathan mentioned, if ideas or changes linger too long, they often are
>>> superceded or replaced with more applicable alternatives or might not have
>>> been that important in the first place. It's a shame because each
>>> languishing PR represents some amount of work from someone (sometimes a
>>> non-trivial amount) but there really isn't a more practical alternative IMO.
>>>
>>>
>>>
>>> On Wed, Dec 15, 2021 at 5:05 PM Jonathan Ellis <jb...@gmail.com> wrote:
>>>
>>>> One problem with the current state is that PRs and even higher level ideas
>>>> have a shelf life. Declaring PR bankruptcy does in fact solve this
>>>> problem.
>>>>
>>>> The other problem is that from a new contributor's perspective it's
>>>> impossible to tell which issues are relevant and which are clutter that we
>>>> haven't gotten around to closing out.
>>>>
>>>> For this, declaring PR bankruptcy isn't as good as somehow having the
>>>> capacity to review and respond to everything, but it's still better than
>>>> the status quo. And since no large-scale OSS project that I'm aware of has
>>>> figured out how to review and respond to everything sustainably, I'd settle
>>>> for an imperfect solution over a perfect one that probably doesn't exist.
>>>>
>>>>
>>>> On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com>
>>>> wrote:
>>>>
>>>>> I'm not convinced by having a blanket policy here.
>>>>>
>>>>> In several cases, these PRs carried some very valuable ideas that
>>>>> still needed some work to get merged. By using blanket close, we'd be
>>>>> losing all that context and we should not do that.
>>>>>
>>>>> What would actually be helpful, is help in reviewing these old PRs to
>>>>> identify what is either already rejected or superseded by other
>>>>> changes and what just needs some help to get completed.
>>>>>
>>>>> Just declaring PR bankrupticity alone won't solve the problem of why
>>>>> more PRs are created than reviewers can review.
>>>>>
>>>>>
>>>>> Matteo
>>>>>
>>>>> --
>>>>> Matteo Merli
>>>>> <ma...@gmail.com>
>>>>>
>>>>> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
>>>>> wrote:
>>>>>>
>>>>>> I am +1 for closing PRs that are over a year old.
>>>>>>
>>>>>> Does anyone else in the community have thoughts on these old PRs?
>>>>>> Getting consensus and creating a process here could help make our
>>>>>> committers more efficient.
>>>>>>
>>>>>> - Michael
>>>>>>
>>>>>>
>>>>>> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com>
>>>> wrote:
>>>>>>>
>>>>>>> Agreed.
>>>>>>>
>>>>>>> I don't think I understand tison's objection to closing very stale
>>>> PRs
>>>>>>> automatically -- if it's gone that long without attention the
>>>> situation
>>>>>>> isn't likely to change. And the submitter can always reopen it if
>>>> it's
>>>>>>> still relevant.
>>>>>>>
>>>>>>> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>>>>>>>
>>>>>>>> I think that any Pulsar committer ought to close any PR that is
>>>> more
>>>>> than
>>>>>>>> one year old. That would clear about 75 from the backlog. The OP
>>>>> should be
>>>>>>>> informed and if they are still interested then they can discuss it
>>>>> here.
>>>>>>>>
>>>>>>>> So when a stale PR is closed we should suggest that the OP
>>>> subscribe
>>>>> to
>>>>>>>> and email dev@pulsar.apache.org to discuss the PR.
>>>>>>>>
>>>>>>>> All the Best,
>>>>>>>> Dave
>>>>>>>> .
>>>>>>>>> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> From my experience, any process won't work. The only way is to
>>>>> inspire
>>>>>>>> more
>>>>>>>>> reviewers act on PRs.
>>>>>>>>>
>>>>>>>>> Instead of talking about how to do it, reviewing one PR now can
>>>>> help the
>>>>>>>>> case.
>>>>>>>>> Also, it's reasonable to close inactive PR if there is a
>>>>> successor. But
>>>>>>>> do
>>>>>>>>> not let
>>>>>>>>> a bot do it, which will create many corner (bad) cases.
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> tison.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>>>>>>>>>
>>>>>>>>>> Hi Pulsar Community,
>>>>>>>>>>
>>>>>>>>>> I am excited to start contributing as a committer! I have a
>>>>> question
>>>>>>>>>> about our process for closing stale PRs.
>>>>>>>>>>
>>>>>>>>>> We have ~300 open PRs right now. Do we have any guidelines on
>>>>> closing
>>>>>>>>>> stale PRs? Of course we don't want to ignore important bug
>>>> fixes,
>>>>> but
>>>>>>>>>> we also don't want to clutter our repo with open PRs that won't
>>>>> get
>>>>>>>> merged.
>>>>>>>>>>
>>>>>>>>>> For example, I reviewed this PR [0] about 3 months ago. The
>>>>>>>>>> contributor has not yet responded to my feedback and it doesn't
>>>>> seem
>>>>>>>>>> to fix an actual bug, so I think it is a candidate for closure.
>>>>> Here
>>>>>>>>>> is another example [1]. I closed this one because it had merge
>>>>>>>>>> conflicts with a commit that fixed the same underlying issue.
>>>>>>>>>>
>>>>>>>>>> Note that our committer guidelines [2] do not provide guidance
>>>> on
>>>>> this
>>>>>>>>>> subject.
>>>>>>>>>>
>>>>>>>>>> [0] - https://github.com/apache/pulsar/pull/11237
>>>>>>>>>> [1] - https://github.com/apache/pulsar/pull/11162
>>>>>>>>>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Michael
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Jonathan Ellis
>>>>>>> co-founder, http://www.datastax.com
>>>>>>> @spyced
>>>>>
>>>>
>>>>
>>>> --
>>>> Jonathan Ellis
>>>> co-founder, http://www.datastax.com
>>>> @spyced
>>>>
>>>
>>>
>>> --
>>>
>>>
>>> Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
>>> www.tibco.com
>>>
>>> <http://www.tibco.com/>
>
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
> On Dec 15, 2021, at 4:06 PM, Matteo Merli <mm...@apache.org> wrote:
>
>> Is #3267 Support set publish time on broker side one of those very valuable ideas that was later rejected, likely for performance reasons?
>
> No, this was one that was superseded by other changes.
Then I’ll close it.
>
>> One problem with the current state is that PRs and even higher level ideas
>> have a shelf life. Declaring PR bankruptcy does in fact solve this problem.
>
> I don't believe that is true in all cases and I absolutely don't
> believe that it is not possible to keep up with the PRs, when the
> reviewing workload is well balanced.
>
> I'm seeing a lot of opinions here, but at the end of the day the
> people doing the hard work of reviewing are always the same few ones.
(1) These are opinions about how to do the work. If you want someone to JFDI it then I’m happy to start closing and labeling as I suggested.
(2) There is a kind of deference being shown to those individuals based on who the contributor selects for review. I wish there was a way for a contributor to ask the dev list for a review.
>
>> Once we have guidance, I am happy to add it to the Committer Guide on
> the wiki [0].
>
> Michael, I agree 100% with that. We should write clear guidelines to
> describe when it makes sense to close, leave for the record, call for
> "help" to continue working on and so on. That will help committers and
> contributors.
>
>> Matteo, your comment raises an additional question for me. What are
>> Apache's rules for completing someone else's contribution? If someone
>> opens a PR to fix a bug, but it is incomplete and they become
>> unresponsive, how can we move their contribution forward? These are
>> the PRs we don't want to close.
>
> I don't think there is any problem in completing someone else's PR,
> provided that:
> * The original author is non-responsive or has no time to work on it
> at the moment (otherwise it would be kind of rude).
> * We give the right credit to the original author (github has good
> support for multiple authorship of a commit)
>
> Continuing with a PR is not very different from merging the WIP and
> fixing it later in a second commit, from a legal perspective.
>
> IANAL, though *AFAIU*, when a contributor is opening a PR is already
> assigning the IP to the ASF. A committer will merge that code (after
> due diligence that it doesn't contain inadmissible code), but the code
> is already "donated to the ASF" at the moment of the PR.
+1.
Regards,
Dave
>
>
> --
> Matteo Merli
> <mm...@apache.org>
>
>
> On Wed, Dec 15, 2021 at 3:14 PM Chris Herzog <ch...@tibco.com.invalid> wrote:
>>
>> It isn't even an issue related to OSS - every long lived project suffers
>> from this same issue. Whether it's a long lingering defect report or a fix
>> that never got integrated in a timely manner, time wounds all heels.
>>
>> Careful considered review is perfection which can't be hit; if it could be
>> done, the situation would never have occured in the first place. Having a
>> time-to-live is pragmatic, not perfect, but pragmatic.
>>
>> As Jonathan mentioned, if ideas or changes linger too long, they often are
>> superceded or replaced with more applicable alternatives or might not have
>> been that important in the first place. It's a shame because each
>> languishing PR represents some amount of work from someone (sometimes a
>> non-trivial amount) but there really isn't a more practical alternative IMO.
>>
>>
>>
>> On Wed, Dec 15, 2021 at 5:05 PM Jonathan Ellis <jb...@gmail.com> wrote:
>>
>>> One problem with the current state is that PRs and even higher level ideas
>>> have a shelf life. Declaring PR bankruptcy does in fact solve this
>>> problem.
>>>
>>> The other problem is that from a new contributor's perspective it's
>>> impossible to tell which issues are relevant and which are clutter that we
>>> haven't gotten around to closing out.
>>>
>>> For this, declaring PR bankruptcy isn't as good as somehow having the
>>> capacity to review and respond to everything, but it's still better than
>>> the status quo. And since no large-scale OSS project that I'm aware of has
>>> figured out how to review and respond to everything sustainably, I'd settle
>>> for an imperfect solution over a perfect one that probably doesn't exist.
>>>
>>>
>>> On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com>
>>> wrote:
>>>
>>>> I'm not convinced by having a blanket policy here.
>>>>
>>>> In several cases, these PRs carried some very valuable ideas that
>>>> still needed some work to get merged. By using blanket close, we'd be
>>>> losing all that context and we should not do that.
>>>>
>>>> What would actually be helpful, is help in reviewing these old PRs to
>>>> identify what is either already rejected or superseded by other
>>>> changes and what just needs some help to get completed.
>>>>
>>>> Just declaring PR bankrupticity alone won't solve the problem of why
>>>> more PRs are created than reviewers can review.
>>>>
>>>>
>>>> Matteo
>>>>
>>>> --
>>>> Matteo Merli
>>>> <ma...@gmail.com>
>>>>
>>>> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
>>>> wrote:
>>>>>
>>>>> I am +1 for closing PRs that are over a year old.
>>>>>
>>>>> Does anyone else in the community have thoughts on these old PRs?
>>>>> Getting consensus and creating a process here could help make our
>>>>> committers more efficient.
>>>>>
>>>>> - Michael
>>>>>
>>>>>
>>>>> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com>
>>> wrote:
>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>> I don't think I understand tison's objection to closing very stale
>>> PRs
>>>>>> automatically -- if it's gone that long without attention the
>>> situation
>>>>>> isn't likely to change. And the submitter can always reopen it if
>>> it's
>>>>>> still relevant.
>>>>>>
>>>>>> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>>>>>>
>>>>>>> I think that any Pulsar committer ought to close any PR that is
>>> more
>>>> than
>>>>>>> one year old. That would clear about 75 from the backlog. The OP
>>>> should be
>>>>>>> informed and if they are still interested then they can discuss it
>>>> here.
>>>>>>>
>>>>>>> So when a stale PR is closed we should suggest that the OP
>>> subscribe
>>>> to
>>>>>>> and email dev@pulsar.apache.org to discuss the PR.
>>>>>>>
>>>>>>> All the Best,
>>>>>>> Dave
>>>>>>> .
>>>>>>>> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> From my experience, any process won't work. The only way is to
>>>> inspire
>>>>>>> more
>>>>>>>> reviewers act on PRs.
>>>>>>>>
>>>>>>>> Instead of talking about how to do it, reviewing one PR now can
>>>> help the
>>>>>>>> case.
>>>>>>>> Also, it's reasonable to close inactive PR if there is a
>>>> successor. But
>>>>>>> do
>>>>>>>> not let
>>>>>>>> a bot do it, which will create many corner (bad) cases.
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> tison.
>>>>>>>>
>>>>>>>>
>>>>>>>> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>>>>>>>>
>>>>>>>>> Hi Pulsar Community,
>>>>>>>>>
>>>>>>>>> I am excited to start contributing as a committer! I have a
>>>> question
>>>>>>>>> about our process for closing stale PRs.
>>>>>>>>>
>>>>>>>>> We have ~300 open PRs right now. Do we have any guidelines on
>>>> closing
>>>>>>>>> stale PRs? Of course we don't want to ignore important bug
>>> fixes,
>>>> but
>>>>>>>>> we also don't want to clutter our repo with open PRs that won't
>>>> get
>>>>>>> merged.
>>>>>>>>>
>>>>>>>>> For example, I reviewed this PR [0] about 3 months ago. The
>>>>>>>>> contributor has not yet responded to my feedback and it doesn't
>>>> seem
>>>>>>>>> to fix an actual bug, so I think it is a candidate for closure.
>>>> Here
>>>>>>>>> is another example [1]. I closed this one because it had merge
>>>>>>>>> conflicts with a commit that fixed the same underlying issue.
>>>>>>>>>
>>>>>>>>> Note that our committer guidelines [2] do not provide guidance
>>> on
>>>> this
>>>>>>>>> subject.
>>>>>>>>>
>>>>>>>>> [0] - https://github.com/apache/pulsar/pull/11237
>>>>>>>>> [1] - https://github.com/apache/pulsar/pull/11162
>>>>>>>>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Michael
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Jonathan Ellis
>>>>>> co-founder, http://www.datastax.com
>>>>>> @spyced
>>>>
>>>
>>>
>>> --
>>> Jonathan Ellis
>>> co-founder, http://www.datastax.com
>>> @spyced
>>>
>>
>>
>> --
>>
>>
>> Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
>> www.tibco.com
>>
>> <http://www.tibco.com/>
Re: [DISCUSS] How to handle stale PRs
Posted by Matteo Merli <mm...@apache.org>.
> Is #3267 Support set publish time on broker side one of those very valuable ideas that was later rejected, likely for performance reasons?
No, this was one that was superseded by other changes.
> One problem with the current state is that PRs and even higher level ideas
> have a shelf life. Declaring PR bankruptcy does in fact solve this problem.
I don't believe that is true in all cases and I absolutely don't
believe that it is not possible to keep up with the PRs, when the
reviewing workload is well balanced.
I'm seeing a lot of opinions here, but at the end of the day the
people doing the hard work of reviewing are always the same few ones.
> Once we have guidance, I am happy to add it to the Committer Guide on
the wiki [0].
Michael, I agree 100% with that. We should write clear guidelines to
describe when it makes sense to close, leave for the record, call for
"help" to continue working on and so on. That will help committers and
contributors.
> Matteo, your comment raises an additional question for me. What are
> Apache's rules for completing someone else's contribution? If someone
> opens a PR to fix a bug, but it is incomplete and they become
> unresponsive, how can we move their contribution forward? These are
> the PRs we don't want to close.
I don't think there is any problem in completing someone else's PR,
provided that:
* The original author is non-responsive or has no time to work on it
at the moment (otherwise it would be kind of rude).
* We give the right credit to the original author (github has good
support for multiple authorship of a commit)
Continuing with a PR is not very different from merging the WIP and
fixing it later in a second commit, from a legal perspective.
IANAL, though *AFAIU*, when a contributor is opening a PR is already
assigning the IP to the ASF. A committer will merge that code (after
due diligence that it doesn't contain inadmissible code), but the code
is already "donated to the ASF" at the moment of the PR.
--
Matteo Merli
<mm...@apache.org>
On Wed, Dec 15, 2021 at 3:14 PM Chris Herzog <ch...@tibco.com.invalid> wrote:
>
> It isn't even an issue related to OSS - every long lived project suffers
> from this same issue. Whether it's a long lingering defect report or a fix
> that never got integrated in a timely manner, time wounds all heels.
>
> Careful considered review is perfection which can't be hit; if it could be
> done, the situation would never have occured in the first place. Having a
> time-to-live is pragmatic, not perfect, but pragmatic.
>
> As Jonathan mentioned, if ideas or changes linger too long, they often are
> superceded or replaced with more applicable alternatives or might not have
> been that important in the first place. It's a shame because each
> languishing PR represents some amount of work from someone (sometimes a
> non-trivial amount) but there really isn't a more practical alternative IMO.
>
>
>
> On Wed, Dec 15, 2021 at 5:05 PM Jonathan Ellis <jb...@gmail.com> wrote:
>
> > One problem with the current state is that PRs and even higher level ideas
> > have a shelf life. Declaring PR bankruptcy does in fact solve this
> > problem.
> >
> > The other problem is that from a new contributor's perspective it's
> > impossible to tell which issues are relevant and which are clutter that we
> > haven't gotten around to closing out.
> >
> > For this, declaring PR bankruptcy isn't as good as somehow having the
> > capacity to review and respond to everything, but it's still better than
> > the status quo. And since no large-scale OSS project that I'm aware of has
> > figured out how to review and respond to everything sustainably, I'd settle
> > for an imperfect solution over a perfect one that probably doesn't exist.
> >
> >
> > On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com>
> > wrote:
> >
> > > I'm not convinced by having a blanket policy here.
> > >
> > > In several cases, these PRs carried some very valuable ideas that
> > > still needed some work to get merged. By using blanket close, we'd be
> > > losing all that context and we should not do that.
> > >
> > > What would actually be helpful, is help in reviewing these old PRs to
> > > identify what is either already rejected or superseded by other
> > > changes and what just needs some help to get completed.
> > >
> > > Just declaring PR bankrupticity alone won't solve the problem of why
> > > more PRs are created than reviewers can review.
> > >
> > >
> > > Matteo
> > >
> > > --
> > > Matteo Merli
> > > <ma...@gmail.com>
> > >
> > > On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
> > > wrote:
> > > >
> > > > I am +1 for closing PRs that are over a year old.
> > > >
> > > > Does anyone else in the community have thoughts on these old PRs?
> > > > Getting consensus and creating a process here could help make our
> > > > committers more efficient.
> > > >
> > > > - Michael
> > > >
> > > >
> > > > On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com>
> > wrote:
> > > > >
> > > > > Agreed.
> > > > >
> > > > > I don't think I understand tison's objection to closing very stale
> > PRs
> > > > > automatically -- if it's gone that long without attention the
> > situation
> > > > > isn't likely to change. And the submitter can always reopen it if
> > it's
> > > > > still relevant.
> > > > >
> > > > > On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> > > > >
> > > > > > I think that any Pulsar committer ought to close any PR that is
> > more
> > > than
> > > > > > one year old. That would clear about 75 from the backlog. The OP
> > > should be
> > > > > > informed and if they are still interested then they can discuss it
> > > here.
> > > > > >
> > > > > > So when a stale PR is closed we should suggest that the OP
> > subscribe
> > > to
> > > > > > and email dev@pulsar.apache.org to discuss the PR.
> > > > > >
> > > > > > All the Best,
> > > > > > Dave
> > > > > > .
> > > > > > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > > > > > >
> > > > > > > From my experience, any process won't work. The only way is to
> > > inspire
> > > > > > more
> > > > > > > reviewers act on PRs.
> > > > > > >
> > > > > > > Instead of talking about how to do it, reviewing one PR now can
> > > help the
> > > > > > > case.
> > > > > > > Also, it's reasonable to close inactive PR if there is a
> > > successor. But
> > > > > > do
> > > > > > > not let
> > > > > > > a bot do it, which will create many corner (bad) cases.
> > > > > > >
> > > > > > > Best,
> > > > > > > tison.
> > > > > > >
> > > > > > >
> > > > > > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > > > > > >
> > > > > > >> Hi Pulsar Community,
> > > > > > >>
> > > > > > >> I am excited to start contributing as a committer! I have a
> > > question
> > > > > > >> about our process for closing stale PRs.
> > > > > > >>
> > > > > > >> We have ~300 open PRs right now. Do we have any guidelines on
> > > closing
> > > > > > >> stale PRs? Of course we don't want to ignore important bug
> > fixes,
> > > but
> > > > > > >> we also don't want to clutter our repo with open PRs that won't
> > > get
> > > > > > merged.
> > > > > > >>
> > > > > > >> For example, I reviewed this PR [0] about 3 months ago. The
> > > > > > >> contributor has not yet responded to my feedback and it doesn't
> > > seem
> > > > > > >> to fix an actual bug, so I think it is a candidate for closure.
> > > Here
> > > > > > >> is another example [1]. I closed this one because it had merge
> > > > > > >> conflicts with a commit that fixed the same underlying issue.
> > > > > > >>
> > > > > > >> Note that our committer guidelines [2] do not provide guidance
> > on
> > > this
> > > > > > >> subject.
> > > > > > >>
> > > > > > >> [0] - https://github.com/apache/pulsar/pull/11237
> > > > > > >> [1] - https://github.com/apache/pulsar/pull/11162
> > > > > > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > > > > > >>
> > > > > > >> Thanks,
> > > > > > >> Michael
> > > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > Jonathan Ellis
> > > > > co-founder, http://www.datastax.com
> > > > > @spyced
> > >
> >
> >
> > --
> > Jonathan Ellis
> > co-founder, http://www.datastax.com
> > @spyced
> >
>
>
> --
>
>
> Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
> www.tibco.com
>
> <http://www.tibco.com/>
Re: [DISCUSS] How to handle stale PRs
Posted by Chris Herzog <ch...@tibco.com.INVALID>.
It isn't even an issue related to OSS - every long lived project suffers
from this same issue. Whether it's a long lingering defect report or a fix
that never got integrated in a timely manner, time wounds all heels.
Careful considered review is perfection which can't be hit; if it could be
done, the situation would never have occured in the first place. Having a
time-to-live is pragmatic, not perfect, but pragmatic.
As Jonathan mentioned, if ideas or changes linger too long, they often are
superceded or replaced with more applicable alternatives or might not have
been that important in the first place. It's a shame because each
languishing PR represents some amount of work from someone (sometimes a
non-trivial amount) but there really isn't a more practical alternative IMO.
On Wed, Dec 15, 2021 at 5:05 PM Jonathan Ellis <jb...@gmail.com> wrote:
> One problem with the current state is that PRs and even higher level ideas
> have a shelf life. Declaring PR bankruptcy does in fact solve this
> problem.
>
> The other problem is that from a new contributor's perspective it's
> impossible to tell which issues are relevant and which are clutter that we
> haven't gotten around to closing out.
>
> For this, declaring PR bankruptcy isn't as good as somehow having the
> capacity to review and respond to everything, but it's still better than
> the status quo. And since no large-scale OSS project that I'm aware of has
> figured out how to review and respond to everything sustainably, I'd settle
> for an imperfect solution over a perfect one that probably doesn't exist.
>
>
> On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com>
> wrote:
>
> > I'm not convinced by having a blanket policy here.
> >
> > In several cases, these PRs carried some very valuable ideas that
> > still needed some work to get merged. By using blanket close, we'd be
> > losing all that context and we should not do that.
> >
> > What would actually be helpful, is help in reviewing these old PRs to
> > identify what is either already rejected or superseded by other
> > changes and what just needs some help to get completed.
> >
> > Just declaring PR bankrupticity alone won't solve the problem of why
> > more PRs are created than reviewers can review.
> >
> >
> > Matteo
> >
> > --
> > Matteo Merli
> > <ma...@gmail.com>
> >
> > On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
> > wrote:
> > >
> > > I am +1 for closing PRs that are over a year old.
> > >
> > > Does anyone else in the community have thoughts on these old PRs?
> > > Getting consensus and creating a process here could help make our
> > > committers more efficient.
> > >
> > > - Michael
> > >
> > >
> > > On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com>
> wrote:
> > > >
> > > > Agreed.
> > > >
> > > > I don't think I understand tison's objection to closing very stale
> PRs
> > > > automatically -- if it's gone that long without attention the
> situation
> > > > isn't likely to change. And the submitter can always reopen it if
> it's
> > > > still relevant.
> > > >
> > > > On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> > > >
> > > > > I think that any Pulsar committer ought to close any PR that is
> more
> > than
> > > > > one year old. That would clear about 75 from the backlog. The OP
> > should be
> > > > > informed and if they are still interested then they can discuss it
> > here.
> > > > >
> > > > > So when a stale PR is closed we should suggest that the OP
> subscribe
> > to
> > > > > and email dev@pulsar.apache.org to discuss the PR.
> > > > >
> > > > > All the Best,
> > > > > Dave
> > > > > .
> > > > > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > > > > >
> > > > > > From my experience, any process won't work. The only way is to
> > inspire
> > > > > more
> > > > > > reviewers act on PRs.
> > > > > >
> > > > > > Instead of talking about how to do it, reviewing one PR now can
> > help the
> > > > > > case.
> > > > > > Also, it's reasonable to close inactive PR if there is a
> > successor. But
> > > > > do
> > > > > > not let
> > > > > > a bot do it, which will create many corner (bad) cases.
> > > > > >
> > > > > > Best,
> > > > > > tison.
> > > > > >
> > > > > >
> > > > > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > > > > >
> > > > > >> Hi Pulsar Community,
> > > > > >>
> > > > > >> I am excited to start contributing as a committer! I have a
> > question
> > > > > >> about our process for closing stale PRs.
> > > > > >>
> > > > > >> We have ~300 open PRs right now. Do we have any guidelines on
> > closing
> > > > > >> stale PRs? Of course we don't want to ignore important bug
> fixes,
> > but
> > > > > >> we also don't want to clutter our repo with open PRs that won't
> > get
> > > > > merged.
> > > > > >>
> > > > > >> For example, I reviewed this PR [0] about 3 months ago. The
> > > > > >> contributor has not yet responded to my feedback and it doesn't
> > seem
> > > > > >> to fix an actual bug, so I think it is a candidate for closure.
> > Here
> > > > > >> is another example [1]. I closed this one because it had merge
> > > > > >> conflicts with a commit that fixed the same underlying issue.
> > > > > >>
> > > > > >> Note that our committer guidelines [2] do not provide guidance
> on
> > this
> > > > > >> subject.
> > > > > >>
> > > > > >> [0] - https://github.com/apache/pulsar/pull/11237
> > > > > >> [1] - https://github.com/apache/pulsar/pull/11162
> > > > > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > > > > >>
> > > > > >> Thanks,
> > > > > >> Michael
> > > > > >>
> > > > >
> > > > >
> > > >
> > > > --
> > > > Jonathan Ellis
> > > > co-founder, http://www.datastax.com
> > > > @spyced
> >
>
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced
>
--
Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
www.tibco.com
<http://www.tibco.com/>
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
I’d like to point out that if we label auto-closed PRs properly it is easy enough to find them in the GitHub UI.
> On Dec 15, 2021, at 3:05 PM, Jonathan Ellis <jb...@gmail.com> wrote:
>
> One problem with the current state is that PRs and even higher level ideas
> have a shelf life. Declaring PR bankruptcy does in fact solve this problem.
>
> The other problem is that from a new contributor's perspective it's
> impossible to tell which issues are relevant and which are clutter that we
> haven't gotten around to closing out.
>
> For this, declaring PR bankruptcy isn't as good as somehow having the
> capacity to review and respond to everything, but it's still better than
> the status quo. And since no large-scale OSS project that I'm aware of has
> figured out how to review and respond to everything sustainably, I'd settle
> for an imperfect solution over a perfect one that probably doesn't exist.
>
>
> On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com> wrote:
>
>> I'm not convinced by having a blanket policy here.
>>
>> In several cases, these PRs carried some very valuable ideas that
>> still needed some work to get merged. By using blanket close, we'd be
>> losing all that context and we should not do that.
>>
>> What would actually be helpful, is help in reviewing these old PRs to
>> identify what is either already rejected or superseded by other
>> changes and what just needs some help to get completed.
>>
>> Just declaring PR bankrupticity alone won't solve the problem of why
>> more PRs are created than reviewers can review.
>>
>>
>> Matteo
>>
>> --
>> Matteo Merli
>> <ma...@gmail.com>
>>
>> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
>> wrote:
>>>
>>> I am +1 for closing PRs that are over a year old.
>>>
>>> Does anyone else in the community have thoughts on these old PRs?
>>> Getting consensus and creating a process here could help make our
>>> committers more efficient.
>>>
>>> - Michael
>>>
>>>
>>> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
>>>>
>>>> Agreed.
>>>>
>>>> I don't think I understand tison's objection to closing very stale PRs
>>>> automatically -- if it's gone that long without attention the situation
>>>> isn't likely to change. And the submitter can always reopen it if it's
>>>> still relevant.
>>>>
>>>> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>>>>
>>>>> I think that any Pulsar committer ought to close any PR that is more
>> than
>>>>> one year old. That would clear about 75 from the backlog. The OP
>> should be
>>>>> informed and if they are still interested then they can discuss it
>> here.
>>>>>
>>>>> So when a stale PR is closed we should suggest that the OP subscribe
>> to
>>>>> and email dev@pulsar.apache.org to discuss the PR.
>>>>>
>>>>> All the Best,
>>>>> Dave
>>>>> .
>>>>>> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>>>>>>
>>>>>> From my experience, any process won't work. The only way is to
>> inspire
>>>>> more
>>>>>> reviewers act on PRs.
>>>>>>
>>>>>> Instead of talking about how to do it, reviewing one PR now can
>> help the
>>>>>> case.
>>>>>> Also, it's reasonable to close inactive PR if there is a
>> successor. But
>>>>> do
>>>>>> not let
>>>>>> a bot do it, which will create many corner (bad) cases.
>>>>>>
>>>>>> Best,
>>>>>> tison.
>>>>>>
>>>>>>
>>>>>> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>>>>>>
>>>>>>> Hi Pulsar Community,
>>>>>>>
>>>>>>> I am excited to start contributing as a committer! I have a
>> question
>>>>>>> about our process for closing stale PRs.
>>>>>>>
>>>>>>> We have ~300 open PRs right now. Do we have any guidelines on
>> closing
>>>>>>> stale PRs? Of course we don't want to ignore important bug fixes,
>> but
>>>>>>> we also don't want to clutter our repo with open PRs that won't
>> get
>>>>> merged.
>>>>>>>
>>>>>>> For example, I reviewed this PR [0] about 3 months ago. The
>>>>>>> contributor has not yet responded to my feedback and it doesn't
>> seem
>>>>>>> to fix an actual bug, so I think it is a candidate for closure.
>> Here
>>>>>>> is another example [1]. I closed this one because it had merge
>>>>>>> conflicts with a commit that fixed the same underlying issue.
>>>>>>>
>>>>>>> Note that our committer guidelines [2] do not provide guidance on
>> this
>>>>>>> subject.
>>>>>>>
>>>>>>> [0] - https://github.com/apache/pulsar/pull/11237
>>>>>>> [1] - https://github.com/apache/pulsar/pull/11162
>>>>>>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Michael
>>>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Jonathan Ellis
>>>> co-founder, http://www.datastax.com
>>>> @spyced
>>
>
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Jonathan Ellis <jb...@gmail.com>.
One problem with the current state is that PRs and even higher level ideas
have a shelf life. Declaring PR bankruptcy does in fact solve this problem.
The other problem is that from a new contributor's perspective it's
impossible to tell which issues are relevant and which are clutter that we
haven't gotten around to closing out.
For this, declaring PR bankruptcy isn't as good as somehow having the
capacity to review and respond to everything, but it's still better than
the status quo. And since no large-scale OSS project that I'm aware of has
figured out how to review and respond to everything sustainably, I'd settle
for an imperfect solution over a perfect one that probably doesn't exist.
On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com> wrote:
> I'm not convinced by having a blanket policy here.
>
> In several cases, these PRs carried some very valuable ideas that
> still needed some work to get merged. By using blanket close, we'd be
> losing all that context and we should not do that.
>
> What would actually be helpful, is help in reviewing these old PRs to
> identify what is either already rejected or superseded by other
> changes and what just needs some help to get completed.
>
> Just declaring PR bankrupticity alone won't solve the problem of why
> more PRs are created than reviewers can review.
>
>
> Matteo
>
> --
> Matteo Merli
> <ma...@gmail.com>
>
> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org>
> wrote:
> >
> > I am +1 for closing PRs that are over a year old.
> >
> > Does anyone else in the community have thoughts on these old PRs?
> > Getting consensus and creating a process here could help make our
> > committers more efficient.
> >
> > - Michael
> >
> >
> > On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
> > >
> > > Agreed.
> > >
> > > I don't think I understand tison's objection to closing very stale PRs
> > > automatically -- if it's gone that long without attention the situation
> > > isn't likely to change. And the submitter can always reopen it if it's
> > > still relevant.
> > >
> > > On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> > >
> > > > I think that any Pulsar committer ought to close any PR that is more
> than
> > > > one year old. That would clear about 75 from the backlog. The OP
> should be
> > > > informed and if they are still interested then they can discuss it
> here.
> > > >
> > > > So when a stale PR is closed we should suggest that the OP subscribe
> to
> > > > and email dev@pulsar.apache.org to discuss the PR.
> > > >
> > > > All the Best,
> > > > Dave
> > > > .
> > > > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > > > >
> > > > > From my experience, any process won't work. The only way is to
> inspire
> > > > more
> > > > > reviewers act on PRs.
> > > > >
> > > > > Instead of talking about how to do it, reviewing one PR now can
> help the
> > > > > case.
> > > > > Also, it's reasonable to close inactive PR if there is a
> successor. But
> > > > do
> > > > > not let
> > > > > a bot do it, which will create many corner (bad) cases.
> > > > >
> > > > > Best,
> > > > > tison.
> > > > >
> > > > >
> > > > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > > > >
> > > > >> Hi Pulsar Community,
> > > > >>
> > > > >> I am excited to start contributing as a committer! I have a
> question
> > > > >> about our process for closing stale PRs.
> > > > >>
> > > > >> We have ~300 open PRs right now. Do we have any guidelines on
> closing
> > > > >> stale PRs? Of course we don't want to ignore important bug fixes,
> but
> > > > >> we also don't want to clutter our repo with open PRs that won't
> get
> > > > merged.
> > > > >>
> > > > >> For example, I reviewed this PR [0] about 3 months ago. The
> > > > >> contributor has not yet responded to my feedback and it doesn't
> seem
> > > > >> to fix an actual bug, so I think it is a candidate for closure.
> Here
> > > > >> is another example [1]. I closed this one because it had merge
> > > > >> conflicts with a commit that fixed the same underlying issue.
> > > > >>
> > > > >> Note that our committer guidelines [2] do not provide guidance on
> this
> > > > >> subject.
> > > > >>
> > > > >> [0] - https://github.com/apache/pulsar/pull/11237
> > > > >> [1] - https://github.com/apache/pulsar/pull/11162
> > > > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > > > >>
> > > > >> Thanks,
> > > > >> Michael
> > > > >>
> > > >
> > > >
> > >
> > > --
> > > Jonathan Ellis
> > > co-founder, http://www.datastax.com
> > > @spyced
>
--
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
I think that there should be a label added to any old PR that a contributor wants to keep open. I think “status/hold” would be a good label name. That will keep others who wish to review old PRs and close them from wasting their time.
I looked at the labels and I wonder about those that are “triage/week-N” Do these have a consistent use case? And if so, what is it?
> On Dec 15, 2021, at 1:11 PM, Matteo Merli <ma...@gmail.com> wrote:
>
> I'm not convinced by having a blanket policy here.
>
> In several cases, these PRs carried some very valuable ideas that
> still needed some work to get merged. By using blanket close, we'd be
> losing all that context and we should not do that.
>
> What would actually be helpful, is help in reviewing these old PRs to
> identify what is either already rejected or superseded by other
> changes and what just needs some help to get completed.
>
> Just declaring PR bankrupticity alone won't solve the problem of why
> more PRs are created than reviewers can review.
>
>
> Matteo
>
> --
> Matteo Merli
> <ma...@gmail.com>
>
> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org> wrote:
>>
>> I am +1 for closing PRs that are over a year old.
>>
>> Does anyone else in the community have thoughts on these old PRs?
>> Getting consensus and creating a process here could help make our
>> committers more efficient.
>>
>> - Michael
>>
>>
>> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
>>>
>>> Agreed.
>>>
>>> I don't think I understand tison's objection to closing very stale PRs
>>> automatically -- if it's gone that long without attention the situation
>>> isn't likely to change. And the submitter can always reopen it if it's
>>> still relevant.
>>>
>>> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>>>
>>>> I think that any Pulsar committer ought to close any PR that is more than
>>>> one year old. That would clear about 75 from the backlog. The OP should be
>>>> informed and if they are still interested then they can discuss it here.
>>>>
>>>> So when a stale PR is closed we should suggest that the OP subscribe to
>>>> and email dev@pulsar.apache.org to discuss the PR.
>>>>
>>>> All the Best,
>>>> Dave
>>>> .
>>>>> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>>>>>
>>>>> From my experience, any process won't work. The only way is to inspire
>>>> more
>>>>> reviewers act on PRs.
>>>>>
>>>>> Instead of talking about how to do it, reviewing one PR now can help the
>>>>> case.
>>>>> Also, it's reasonable to close inactive PR if there is a successor. But
>>>> do
>>>>> not let
>>>>> a bot do it, which will create many corner (bad) cases.
>>>>>
>>>>> Best,
>>>>> tison.
>>>>>
>>>>>
>>>>> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>>>>>
>>>>>> Hi Pulsar Community,
>>>>>>
>>>>>> I am excited to start contributing as a committer! I have a question
>>>>>> about our process for closing stale PRs.
>>>>>>
>>>>>> We have ~300 open PRs right now. Do we have any guidelines on closing
>>>>>> stale PRs? Of course we don't want to ignore important bug fixes, but
>>>>>> we also don't want to clutter our repo with open PRs that won't get
>>>> merged.
>>>>>>
>>>>>> For example, I reviewed this PR [0] about 3 months ago. The
>>>>>> contributor has not yet responded to my feedback and it doesn't seem
>>>>>> to fix an actual bug, so I think it is a candidate for closure. Here
>>>>>> is another example [1]. I closed this one because it had merge
>>>>>> conflicts with a commit that fixed the same underlying issue.
>>>>>>
>>>>>> Note that our committer guidelines [2] do not provide guidance on this
>>>>>> subject.
>>>>>>
>>>>>> [0] - https://github.com/apache/pulsar/pull/11237
>>>>>> [1] - https://github.com/apache/pulsar/pull/11162
>>>>>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>>>>>
>>>>>> Thanks,
>>>>>> Michael
>>>>>>
>>>>
>>>>
>>>
>>> --
>>> Jonathan Ellis
>>> co-founder, http://www.datastax.com
>>> @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
> On Dec 15, 2021, at 1:11 PM, Matteo Merli <ma...@gmail.com> wrote:
>
> I'm not convinced by having a blanket policy here.
>
> In several cases, these PRs carried some very valuable ideas that
> still needed some work to get merged. By using blanket close, we'd be
> losing all that context and we should not do that.
Is #3267 Support set publish time on broker side one of those very valuable ideas that was later rejected, likely for performance reasons?
>
> What would actually be helpful, is help in reviewing these old PRs to
> identify what is either already rejected or superseded by other
> changes and what just needs some help to get completed.
>
> Just declaring PR bankrupticity alone won't solve the problem of why
> more PRs are created than reviewers can review.
>
>
> Matteo
>
> --
> Matteo Merli
> <ma...@gmail.com>
>
> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org> wrote:
>>
>> I am +1 for closing PRs that are over a year old.
>>
>> Does anyone else in the community have thoughts on these old PRs?
>> Getting consensus and creating a process here could help make our
>> committers more efficient.
>>
>> - Michael
>>
>>
>> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
>>>
>>> Agreed.
>>>
>>> I don't think I understand tison's objection to closing very stale PRs
>>> automatically -- if it's gone that long without attention the situation
>>> isn't likely to change. And the submitter can always reopen it if it's
>>> still relevant.
>>>
>>> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>>>
>>>> I think that any Pulsar committer ought to close any PR that is more than
>>>> one year old. That would clear about 75 from the backlog. The OP should be
>>>> informed and if they are still interested then they can discuss it here.
>>>>
>>>> So when a stale PR is closed we should suggest that the OP subscribe to
>>>> and email dev@pulsar.apache.org to discuss the PR.
>>>>
>>>> All the Best,
>>>> Dave
>>>> .
>>>>> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>>>>>
>>>>> From my experience, any process won't work. The only way is to inspire
>>>> more
>>>>> reviewers act on PRs.
>>>>>
>>>>> Instead of talking about how to do it, reviewing one PR now can help the
>>>>> case.
>>>>> Also, it's reasonable to close inactive PR if there is a successor. But
>>>> do
>>>>> not let
>>>>> a bot do it, which will create many corner (bad) cases.
>>>>>
>>>>> Best,
>>>>> tison.
>>>>>
>>>>>
>>>>> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>>>>>
>>>>>> Hi Pulsar Community,
>>>>>>
>>>>>> I am excited to start contributing as a committer! I have a question
>>>>>> about our process for closing stale PRs.
>>>>>>
>>>>>> We have ~300 open PRs right now. Do we have any guidelines on closing
>>>>>> stale PRs? Of course we don't want to ignore important bug fixes, but
>>>>>> we also don't want to clutter our repo with open PRs that won't get
>>>> merged.
>>>>>>
>>>>>> For example, I reviewed this PR [0] about 3 months ago. The
>>>>>> contributor has not yet responded to my feedback and it doesn't seem
>>>>>> to fix an actual bug, so I think it is a candidate for closure. Here
>>>>>> is another example [1]. I closed this one because it had merge
>>>>>> conflicts with a commit that fixed the same underlying issue.
>>>>>>
>>>>>> Note that our committer guidelines [2] do not provide guidance on this
>>>>>> subject.
>>>>>>
>>>>>> [0] - https://github.com/apache/pulsar/pull/11237
>>>>>> [1] - https://github.com/apache/pulsar/pull/11162
>>>>>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>>>>>
>>>>>> Thanks,
>>>>>> Michael
>>>>>>
>>>>
>>>>
>>>
>>> --
>>> Jonathan Ellis
>>> co-founder, http://www.datastax.com
>>> @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Michael Marshall <mm...@apache.org>.
> I'm not convinced by having a blanket policy here.
I'm fine with stopping short of an automated policy. However, I think
it'd be helpful to provide committers, especially new committers, with
conditions that make a PR eligible to be closed. Since committers act
on behalf of the PMC, documentation can give them confidence that they
are acting correctly.
Once we have guidance, I am happy to add it to the Committer Guide on
the wiki [0].
For example, we could say that a committer can close a PR when:
1. The PR's intended change is superseded by another change. In this
case, the committer should provide a reference to the other PR.
2. The PR's intended change has been rejected by the community. In
this case, the rejection must first be discussed on the mailing list.
3. The PR does not appear to fix an issue and the contributor has not
responded to a request for clarification within some time frame (maybe
a month?). In this case, the committer should note that the PR can be
reopened if/when the contributor is available to move the PR forward.
My goal in starting this thread was to clarify when I can close PRs.
Matteo, your comment raises an additional question for me. What are
Apache's rules for completing someone else's contribution? If someone
opens a PR to fix a bug, but it is incomplete and they become
unresponsive, how can we move their contribution forward? These are
the PRs we don't want to close.
Thanks,
Michael
[0] https://github.com/apache/pulsar/wiki/Committer-Guide
On Wed, Dec 15, 2021 at 3:12 PM Matteo Merli <ma...@gmail.com> wrote:
>
> I'm not convinced by having a blanket policy here.
>
> In several cases, these PRs carried some very valuable ideas that
> still needed some work to get merged. By using blanket close, we'd be
> losing all that context and we should not do that.
>
> What would actually be helpful, is help in reviewing these old PRs to
> identify what is either already rejected or superseded by other
> changes and what just needs some help to get completed.
>
> Just declaring PR bankrupticity alone won't solve the problem of why
> more PRs are created than reviewers can review.
>
>
> Matteo
>
> --
> Matteo Merli
> <ma...@gmail.com>
>
> On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org> wrote:
> >
> > I am +1 for closing PRs that are over a year old.
> >
> > Does anyone else in the community have thoughts on these old PRs?
> > Getting consensus and creating a process here could help make our
> > committers more efficient.
> >
> > - Michael
> >
> >
> > On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
> > >
> > > Agreed.
> > >
> > > I don't think I understand tison's objection to closing very stale PRs
> > > automatically -- if it's gone that long without attention the situation
> > > isn't likely to change. And the submitter can always reopen it if it's
> > > still relevant.
> > >
> > > On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> > >
> > > > I think that any Pulsar committer ought to close any PR that is more than
> > > > one year old. That would clear about 75 from the backlog. The OP should be
> > > > informed and if they are still interested then they can discuss it here.
> > > >
> > > > So when a stale PR is closed we should suggest that the OP subscribe to
> > > > and email dev@pulsar.apache.org to discuss the PR.
> > > >
> > > > All the Best,
> > > > Dave
> > > > .
> > > > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > > > >
> > > > > From my experience, any process won't work. The only way is to inspire
> > > > more
> > > > > reviewers act on PRs.
> > > > >
> > > > > Instead of talking about how to do it, reviewing one PR now can help the
> > > > > case.
> > > > > Also, it's reasonable to close inactive PR if there is a successor. But
> > > > do
> > > > > not let
> > > > > a bot do it, which will create many corner (bad) cases.
> > > > >
> > > > > Best,
> > > > > tison.
> > > > >
> > > > >
> > > > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > > > >
> > > > >> Hi Pulsar Community,
> > > > >>
> > > > >> I am excited to start contributing as a committer! I have a question
> > > > >> about our process for closing stale PRs.
> > > > >>
> > > > >> We have ~300 open PRs right now. Do we have any guidelines on closing
> > > > >> stale PRs? Of course we don't want to ignore important bug fixes, but
> > > > >> we also don't want to clutter our repo with open PRs that won't get
> > > > merged.
> > > > >>
> > > > >> For example, I reviewed this PR [0] about 3 months ago. The
> > > > >> contributor has not yet responded to my feedback and it doesn't seem
> > > > >> to fix an actual bug, so I think it is a candidate for closure. Here
> > > > >> is another example [1]. I closed this one because it had merge
> > > > >> conflicts with a commit that fixed the same underlying issue.
> > > > >>
> > > > >> Note that our committer guidelines [2] do not provide guidance on this
> > > > >> subject.
> > > > >>
> > > > >> [0] - https://github.com/apache/pulsar/pull/11237
> > > > >> [1] - https://github.com/apache/pulsar/pull/11162
> > > > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > > > >>
> > > > >> Thanks,
> > > > >> Michael
> > > > >>
> > > >
> > > >
> > >
> > > --
> > > Jonathan Ellis
> > > co-founder, http://www.datastax.com
> > > @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Matteo Merli <ma...@gmail.com>.
I'm not convinced by having a blanket policy here.
In several cases, these PRs carried some very valuable ideas that
still needed some work to get merged. By using blanket close, we'd be
losing all that context and we should not do that.
What would actually be helpful, is help in reviewing these old PRs to
identify what is either already rejected or superseded by other
changes and what just needs some help to get completed.
Just declaring PR bankrupticity alone won't solve the problem of why
more PRs are created than reviewers can review.
Matteo
--
Matteo Merli
<ma...@gmail.com>
On Wed, Dec 15, 2021 at 1:05 PM Michael Marshall <mm...@apache.org> wrote:
>
> I am +1 for closing PRs that are over a year old.
>
> Does anyone else in the community have thoughts on these old PRs?
> Getting consensus and creating a process here could help make our
> committers more efficient.
>
> - Michael
>
>
> On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
> >
> > Agreed.
> >
> > I don't think I understand tison's objection to closing very stale PRs
> > automatically -- if it's gone that long without attention the situation
> > isn't likely to change. And the submitter can always reopen it if it's
> > still relevant.
> >
> > On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> >
> > > I think that any Pulsar committer ought to close any PR that is more than
> > > one year old. That would clear about 75 from the backlog. The OP should be
> > > informed and if they are still interested then they can discuss it here.
> > >
> > > So when a stale PR is closed we should suggest that the OP subscribe to
> > > and email dev@pulsar.apache.org to discuss the PR.
> > >
> > > All the Best,
> > > Dave
> > > .
> > > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > > >
> > > > From my experience, any process won't work. The only way is to inspire
> > > more
> > > > reviewers act on PRs.
> > > >
> > > > Instead of talking about how to do it, reviewing one PR now can help the
> > > > case.
> > > > Also, it's reasonable to close inactive PR if there is a successor. But
> > > do
> > > > not let
> > > > a bot do it, which will create many corner (bad) cases.
> > > >
> > > > Best,
> > > > tison.
> > > >
> > > >
> > > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > > >
> > > >> Hi Pulsar Community,
> > > >>
> > > >> I am excited to start contributing as a committer! I have a question
> > > >> about our process for closing stale PRs.
> > > >>
> > > >> We have ~300 open PRs right now. Do we have any guidelines on closing
> > > >> stale PRs? Of course we don't want to ignore important bug fixes, but
> > > >> we also don't want to clutter our repo with open PRs that won't get
> > > merged.
> > > >>
> > > >> For example, I reviewed this PR [0] about 3 months ago. The
> > > >> contributor has not yet responded to my feedback and it doesn't seem
> > > >> to fix an actual bug, so I think it is a candidate for closure. Here
> > > >> is another example [1]. I closed this one because it had merge
> > > >> conflicts with a commit that fixed the same underlying issue.
> > > >>
> > > >> Note that our committer guidelines [2] do not provide guidance on this
> > > >> subject.
> > > >>
> > > >> [0] - https://github.com/apache/pulsar/pull/11237
> > > >> [1] - https://github.com/apache/pulsar/pull/11162
> > > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > > >>
> > > >> Thanks,
> > > >> Michael
> > > >>
> > >
> > >
> >
> > --
> > Jonathan Ellis
> > co-founder, http://www.datastax.com
> > @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Michael Marshall <mm...@apache.org>.
I am +1 for closing PRs that are over a year old.
Does anyone else in the community have thoughts on these old PRs?
Getting consensus and creating a process here could help make our
committers more efficient.
- Michael
On Fri, Dec 3, 2021 at 1:25 PM Jonathan Ellis <jb...@gmail.com> wrote:
>
> Agreed.
>
> I don't think I understand tison's objection to closing very stale PRs
> automatically -- if it's gone that long without attention the situation
> isn't likely to change. And the submitter can always reopen it if it's
> still relevant.
>
> On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
>
> > I think that any Pulsar committer ought to close any PR that is more than
> > one year old. That would clear about 75 from the backlog. The OP should be
> > informed and if they are still interested then they can discuss it here.
> >
> > So when a stale PR is closed we should suggest that the OP subscribe to
> > and email dev@pulsar.apache.org to discuss the PR.
> >
> > All the Best,
> > Dave
> > .
> > > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> > >
> > > From my experience, any process won't work. The only way is to inspire
> > more
> > > reviewers act on PRs.
> > >
> > > Instead of talking about how to do it, reviewing one PR now can help the
> > > case.
> > > Also, it's reasonable to close inactive PR if there is a successor. But
> > do
> > > not let
> > > a bot do it, which will create many corner (bad) cases.
> > >
> > > Best,
> > > tison.
> > >
> > >
> > > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> > >
> > >> Hi Pulsar Community,
> > >>
> > >> I am excited to start contributing as a committer! I have a question
> > >> about our process for closing stale PRs.
> > >>
> > >> We have ~300 open PRs right now. Do we have any guidelines on closing
> > >> stale PRs? Of course we don't want to ignore important bug fixes, but
> > >> we also don't want to clutter our repo with open PRs that won't get
> > merged.
> > >>
> > >> For example, I reviewed this PR [0] about 3 months ago. The
> > >> contributor has not yet responded to my feedback and it doesn't seem
> > >> to fix an actual bug, so I think it is a candidate for closure. Here
> > >> is another example [1]. I closed this one because it had merge
> > >> conflicts with a commit that fixed the same underlying issue.
> > >>
> > >> Note that our committer guidelines [2] do not provide guidance on this
> > >> subject.
> > >>
> > >> [0] - https://github.com/apache/pulsar/pull/11237
> > >> [1] - https://github.com/apache/pulsar/pull/11162
> > >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> > >>
> > >> Thanks,
> > >> Michael
> > >>
> >
> >
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Jonathan Ellis <jb...@gmail.com>.
Agreed.
I don't think I understand tison's objection to closing very stale PRs
automatically -- if it's gone that long without attention the situation
isn't likely to change. And the submitter can always reopen it if it's
still relevant.
On Fri, Dec 3, 2021 at 1:17 PM Dave Fisher <wa...@apache.org> wrote:
> I think that any Pulsar committer ought to close any PR that is more than
> one year old. That would clear about 75 from the backlog. The OP should be
> informed and if they are still interested then they can discuss it here.
>
> So when a stale PR is closed we should suggest that the OP subscribe to
> and email dev@pulsar.apache.org to discuss the PR.
>
> All the Best,
> Dave
> .
> > On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
> >
> > From my experience, any process won't work. The only way is to inspire
> more
> > reviewers act on PRs.
> >
> > Instead of talking about how to do it, reviewing one PR now can help the
> > case.
> > Also, it's reasonable to close inactive PR if there is a successor. But
> do
> > not let
> > a bot do it, which will create many corner (bad) cases.
> >
> > Best,
> > tison.
> >
> >
> > Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> >
> >> Hi Pulsar Community,
> >>
> >> I am excited to start contributing as a committer! I have a question
> >> about our process for closing stale PRs.
> >>
> >> We have ~300 open PRs right now. Do we have any guidelines on closing
> >> stale PRs? Of course we don't want to ignore important bug fixes, but
> >> we also don't want to clutter our repo with open PRs that won't get
> merged.
> >>
> >> For example, I reviewed this PR [0] about 3 months ago. The
> >> contributor has not yet responded to my feedback and it doesn't seem
> >> to fix an actual bug, so I think it is a candidate for closure. Here
> >> is another example [1]. I closed this one because it had merge
> >> conflicts with a commit that fixed the same underlying issue.
> >>
> >> Note that our committer guidelines [2] do not provide guidance on this
> >> subject.
> >>
> >> [0] - https://github.com/apache/pulsar/pull/11237
> >> [1] - https://github.com/apache/pulsar/pull/11162
> >> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
> >>
> >> Thanks,
> >> Michael
> >>
>
>
--
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced
Re: [DISCUSS] How to handle stale PRs
Posted by Dave Fisher <wa...@apache.org>.
I think that any Pulsar committer ought to close any PR that is more than one year old. That would clear about 75 from the backlog. The OP should be informed and if they are still interested then they can discuss it here.
So when a stale PR is closed we should suggest that the OP subscribe to and email dev@pulsar.apache.org to discuss the PR.
All the Best,
Dave
.
> On Dec 3, 2021, at 9:17 AM, tison <wa...@gmail.com> wrote:
>
> From my experience, any process won't work. The only way is to inspire more
> reviewers act on PRs.
>
> Instead of talking about how to do it, reviewing one PR now can help the
> case.
> Also, it's reasonable to close inactive PR if there is a successor. But do
> not let
> a bot do it, which will create many corner (bad) cases.
>
> Best,
> tison.
>
>
> Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
>
>> Hi Pulsar Community,
>>
>> I am excited to start contributing as a committer! I have a question
>> about our process for closing stale PRs.
>>
>> We have ~300 open PRs right now. Do we have any guidelines on closing
>> stale PRs? Of course we don't want to ignore important bug fixes, but
>> we also don't want to clutter our repo with open PRs that won't get merged.
>>
>> For example, I reviewed this PR [0] about 3 months ago. The
>> contributor has not yet responded to my feedback and it doesn't seem
>> to fix an actual bug, so I think it is a candidate for closure. Here
>> is another example [1]. I closed this one because it had merge
>> conflicts with a commit that fixed the same underlying issue.
>>
>> Note that our committer guidelines [2] do not provide guidance on this
>> subject.
>>
>> [0] - https://github.com/apache/pulsar/pull/11237
>> [1] - https://github.com/apache/pulsar/pull/11162
>> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>>
>> Thanks,
>> Michael
>>
Re: [DISCUSS] How to handle stale PRs
Posted by tison <wa...@gmail.com>.
From my experience, any process won't work. The only way is to inspire more
reviewers act on PRs.
Instead of talking about how to do it, reviewing one PR now can help the
case.
Also, it's reasonable to close inactive PR if there is a successor. But do
not let
a bot do it, which will create many corner (bad) cases.
Best,
tison.
Michael Marshall <mm...@apache.org> 于2021年12月4日周六 00:57写道：
> Hi Pulsar Community,
>
> I am excited to start contributing as a committer! I have a question
> about our process for closing stale PRs.
>
> We have ~300 open PRs right now. Do we have any guidelines on closing
> stale PRs? Of course we don't want to ignore important bug fixes, but
> we also don't want to clutter our repo with open PRs that won't get merged.
>
> For example, I reviewed this PR [0] about 3 months ago. The
> contributor has not yet responded to my feedback and it doesn't seem
> to fix an actual bug, so I think it is a candidate for closure. Here
> is another example [1]. I closed this one because it had merge
> conflicts with a commit that fixed the same underlying issue.
>
> Note that our committer guidelines [2] do not provide guidance on this
> subject.
>
> [0] - https://github.com/apache/pulsar/pull/11237
> [1] - https://github.com/apache/pulsar/pull/11162
> [2] - https://github.com/apache/pulsar/wiki/Committer-Guide
>
> Thanks,
> Michael
>