You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Mridul Muralidharan <mr...@gmail.com> on 2015/12/31 04:00:54 UTC

Automated close of PR's ?

Is there a script running to close "old" PR's ? I was not aware of any
discussion about this in dev list.

- Mridul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Re: Automated close of PR's ?

Posted by Sean Owen <so...@cloudera.com>.
Yes, I mean "open" for a long time, but I do mean PRs aren't intended
to be open for long periods. Of course, they actually stick around
forever on github.

I think Reynold did manually close yours, but I was noting for the
record that there's also an automated process that does this in
response to a request. That has also surprised people in the past.

Generally, we have way more problem with people abandoning or failing
to follow through on PRs, or simply proposing things that aren't going
to be merged. I agree in general with reflecting the reality by
closing lots more JIRAs and PRs -- mostly because these are not
permanent operations at all, and the intent is that in the occasional
case where the owner disagrees, it can simply be reopened. This serves
as a reminder that we need to drive all of these things to a
conclusion.

On Thu, Dec 31, 2015 at 8:59 AM, Mridul Muralidharan <mr...@gmail.com> wrote:
> On the contrary, PR's are actually meant to be long lived - and a
> reference of discussion about review and changes.
> Particularly so for spark since JIRA's and review board are not used
> for code review.
> Note - they are not used only in spark, but by other organization's to
> track contributions (like in our case).
>
> If you look at Reynold's response, he has clarified they were closed
> by him and not via user request - he probably missed out on activity
> on some of the PR's when closing in bulk.
> I would have preferred pinging the PR contributors to close, and
> subsequently doing so if inactive after "some" time (and definitely
> not when folks are off on vacations).
>
> Regards,
> Mridul
>
>
>
> On Thu, Dec 31, 2015 at 12:02 AM, Sean Owen <so...@cloudera.com> wrote:
>> There's a script that can be run manually which closes PRs that have
>> been 'requested' to be closed. I'm not sure of the exact words it
>> looks for but "Do you mind closing this PR?" seems to work. However it
>> does seem to mean that PRs will occasionally get closed as a false
>> positive, so maybe that happened here.
>>
>> You can use your judgment about whether to reopen, but I tend to think
>> PRs are not meant to be long-lived. They don't go away even when
>> closed, so can always stand as a record of a proposed change or be
>> reopened. But there shouldn't be such a thing as a PR open for months.
>> (In practice, you can see a huge number of dead, stale PRs are left
>> open by people out there anyway)
>>
>> On Thu, Dec 31, 2015 at 3:25 AM, Mridul Muralidharan <mr...@gmail.com> wrote:
>>> I am not sure of others, but I had a PR close from under me where
>>> ongoing discussion was as late as 2 weeks back.
>>> Given this, I assumed it was automated close and not manual !
>>>
>>> When the change was opened is not a good metric about viability of the
>>> change (particularly when it touches code which is rarely modified;
>>> and so will merge after multiple releases).
>>>
>>> Regards,
>>> Mridul
>>>
>>> On Wed, Dec 30, 2015 at 7:12 PM, Reynold Xin <rx...@databricks.com> wrote:
>>>> No there is not. I actually manually closed them to cut down the number of
>>>> open pull requests. Feel free to reopen individual ones.
>>>>
>>>>
>>>> On Wednesday, December 30, 2015, Mridul Muralidharan <mr...@gmail.com>
>>>> wrote:
>>>>>
>>>>> Is there a script running to close "old" PR's ? I was not aware of any
>>>>> discussion about this in dev list.
>>>>>
>>>>> - Mridul
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>>>>> For additional commands, e-mail: dev-help@spark.apache.org
>>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>>> For additional commands, e-mail: dev-help@spark.apache.org
>>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Re: Automated close of PR's ?

Posted by Reynold Xin <rx...@databricks.com>.
Hi Mridul,

Thanks for the message. All you said made sense to me.

It can definitely be frustrating when one of your pull requests got
accidentally closed, and that's why we often ping the original authors to
close them. However, this doesn't always work because the original authors
might be inactive for some old pull requests, and sorting these out of 400+
open pull requests can be pretty tough.

Regardless of whether pull requests should be long-lived or short-lived,
closing a pull request does not wipe any of its content. All the changes
and their associated reviews are kept there, and it is trivial to re-open
(one click of a button).

I also agree with your point that the "date of open" is not the best metric
to look at here. Inactivity for a certain period is a much better metric to
use in the future.


On Thu, Dec 31, 2015 at 12:59 AM, Mridul Muralidharan <mr...@gmail.com>
wrote:

> On the contrary, PR's are actually meant to be long lived - and a
> reference of discussion about review and changes.
> Particularly so for spark since JIRA's and review board are not used
> for code review.
> Note - they are not used only in spark, but by other organization's to
> track contributions (like in our case).
>
> If you look at Reynold's response, he has clarified they were closed
> by him and not via user request - he probably missed out on activity
> on some of the PR's when closing in bulk.
> I would have preferred pinging the PR contributors to close, and
> subsequently doing so if inactive after "some" time (and definitely
> not when folks are off on vacations).
>
> Regards,
> Mridul
>
>
>
> On Thu, Dec 31, 2015 at 12:02 AM, Sean Owen <so...@cloudera.com> wrote:
> > There's a script that can be run manually which closes PRs that have
> > been 'requested' to be closed. I'm not sure of the exact words it
> > looks for but "Do you mind closing this PR?" seems to work. However it
> > does seem to mean that PRs will occasionally get closed as a false
> > positive, so maybe that happened here.
> >
> > You can use your judgment about whether to reopen, but I tend to think
> > PRs are not meant to be long-lived. They don't go away even when
> > closed, so can always stand as a record of a proposed change or be
> > reopened. But there shouldn't be such a thing as a PR open for months.
> > (In practice, you can see a huge number of dead, stale PRs are left
> > open by people out there anyway)
> >
> > On Thu, Dec 31, 2015 at 3:25 AM, Mridul Muralidharan <mr...@gmail.com>
> wrote:
> >> I am not sure of others, but I had a PR close from under me where
> >> ongoing discussion was as late as 2 weeks back.
> >> Given this, I assumed it was automated close and not manual !
> >>
> >> When the change was opened is not a good metric about viability of the
> >> change (particularly when it touches code which is rarely modified;
> >> and so will merge after multiple releases).
> >>
> >> Regards,
> >> Mridul
> >>
> >> On Wed, Dec 30, 2015 at 7:12 PM, Reynold Xin <rx...@databricks.com>
> wrote:
> >>> No there is not. I actually manually closed them to cut down the
> number of
> >>> open pull requests. Feel free to reopen individual ones.
> >>>
> >>>
> >>> On Wednesday, December 30, 2015, Mridul Muralidharan <mridul@gmail.com
> >
> >>> wrote:
> >>>>
> >>>> Is there a script running to close "old" PR's ? I was not aware of any
> >>>> discussion about this in dev list.
> >>>>
> >>>> - Mridul
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
> >>>> For additional commands, e-mail: dev-help@spark.apache.org
> >>>>
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
> >> For additional commands, e-mail: dev-help@spark.apache.org
> >>
>

Re: Automated close of PR's ?

Posted by Mridul Muralidharan <mr...@gmail.com>.
On the contrary, PR's are actually meant to be long lived - and a
reference of discussion about review and changes.
Particularly so for spark since JIRA's and review board are not used
for code review.
Note - they are not used only in spark, but by other organization's to
track contributions (like in our case).

If you look at Reynold's response, he has clarified they were closed
by him and not via user request - he probably missed out on activity
on some of the PR's when closing in bulk.
I would have preferred pinging the PR contributors to close, and
subsequently doing so if inactive after "some" time (and definitely
not when folks are off on vacations).

Regards,
Mridul



On Thu, Dec 31, 2015 at 12:02 AM, Sean Owen <so...@cloudera.com> wrote:
> There's a script that can be run manually which closes PRs that have
> been 'requested' to be closed. I'm not sure of the exact words it
> looks for but "Do you mind closing this PR?" seems to work. However it
> does seem to mean that PRs will occasionally get closed as a false
> positive, so maybe that happened here.
>
> You can use your judgment about whether to reopen, but I tend to think
> PRs are not meant to be long-lived. They don't go away even when
> closed, so can always stand as a record of a proposed change or be
> reopened. But there shouldn't be such a thing as a PR open for months.
> (In practice, you can see a huge number of dead, stale PRs are left
> open by people out there anyway)
>
> On Thu, Dec 31, 2015 at 3:25 AM, Mridul Muralidharan <mr...@gmail.com> wrote:
>> I am not sure of others, but I had a PR close from under me where
>> ongoing discussion was as late as 2 weeks back.
>> Given this, I assumed it was automated close and not manual !
>>
>> When the change was opened is not a good metric about viability of the
>> change (particularly when it touches code which is rarely modified;
>> and so will merge after multiple releases).
>>
>> Regards,
>> Mridul
>>
>> On Wed, Dec 30, 2015 at 7:12 PM, Reynold Xin <rx...@databricks.com> wrote:
>>> No there is not. I actually manually closed them to cut down the number of
>>> open pull requests. Feel free to reopen individual ones.
>>>
>>>
>>> On Wednesday, December 30, 2015, Mridul Muralidharan <mr...@gmail.com>
>>> wrote:
>>>>
>>>> Is there a script running to close "old" PR's ? I was not aware of any
>>>> discussion about this in dev list.
>>>>
>>>> - Mridul
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>>>> For additional commands, e-mail: dev-help@spark.apache.org
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>> For additional commands, e-mail: dev-help@spark.apache.org
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Re: Automated close of PR's ?

Posted by Sean Owen <so...@cloudera.com>.
There's a script that can be run manually which closes PRs that have
been 'requested' to be closed. I'm not sure of the exact words it
looks for but "Do you mind closing this PR?" seems to work. However it
does seem to mean that PRs will occasionally get closed as a false
positive, so maybe that happened here.

You can use your judgment about whether to reopen, but I tend to think
PRs are not meant to be long-lived. They don't go away even when
closed, so can always stand as a record of a proposed change or be
reopened. But there shouldn't be such a thing as a PR open for months.
(In practice, you can see a huge number of dead, stale PRs are left
open by people out there anyway)

On Thu, Dec 31, 2015 at 3:25 AM, Mridul Muralidharan <mr...@gmail.com> wrote:
> I am not sure of others, but I had a PR close from under me where
> ongoing discussion was as late as 2 weeks back.
> Given this, I assumed it was automated close and not manual !
>
> When the change was opened is not a good metric about viability of the
> change (particularly when it touches code which is rarely modified;
> and so will merge after multiple releases).
>
> Regards,
> Mridul
>
> On Wed, Dec 30, 2015 at 7:12 PM, Reynold Xin <rx...@databricks.com> wrote:
>> No there is not. I actually manually closed them to cut down the number of
>> open pull requests. Feel free to reopen individual ones.
>>
>>
>> On Wednesday, December 30, 2015, Mridul Muralidharan <mr...@gmail.com>
>> wrote:
>>>
>>> Is there a script running to close "old" PR's ? I was not aware of any
>>> discussion about this in dev list.
>>>
>>> - Mridul
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>>> For additional commands, e-mail: dev-help@spark.apache.org
>>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
> For additional commands, e-mail: dev-help@spark.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Re: Automated close of PR's ?

Posted by Mridul Muralidharan <mr...@gmail.com>.
I am not sure of others, but I had a PR close from under me where
ongoing discussion was as late as 2 weeks back.
Given this, I assumed it was automated close and not manual !

When the change was opened is not a good metric about viability of the
change (particularly when it touches code which is rarely modified;
and so will merge after multiple releases).

Regards,
Mridul

On Wed, Dec 30, 2015 at 7:12 PM, Reynold Xin <rx...@databricks.com> wrote:
> No there is not. I actually manually closed them to cut down the number of
> open pull requests. Feel free to reopen individual ones.
>
>
> On Wednesday, December 30, 2015, Mridul Muralidharan <mr...@gmail.com>
> wrote:
>>
>> Is there a script running to close "old" PR's ? I was not aware of any
>> discussion about this in dev list.
>>
>> - Mridul
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
>> For additional commands, e-mail: dev-help@spark.apache.org
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Re: Automated close of PR's ?

Posted by Reynold Xin <rx...@databricks.com>.
No there is not. I actually manually closed them to cut down the number of
open pull requests. Feel free to reopen individual ones.

On Wednesday, December 30, 2015, Mridul Muralidharan <mr...@gmail.com>
wrote:

> Is there a script running to close "old" PR's ? I was not aware of any
> discussion about this in dev list.
>
> - Mridul
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org <javascript:;>
> For additional commands, e-mail: dev-help@spark.apache.org <javascript:;>
>
>