On 1/16/15 9:41 AM, Tobias Pankrath wrote:
> That reply was not about my pull request specifically. Since it
> basically consists of two new files, it can stay there for months
> without generating any additional work for me. But it is a good
> counterexample to the »just close old stuff«-policy.

I agree that a hamfisted policy would do more harm than good. That's why it's so hard to define!

I'm thinking of something like: if there's $(legitimate) request for changes but the author is dormant for more than $(X) days, then close.

Andrei


Macros:

legitimate=?
X=?

> I agree that a hamfisted policy would do more harm than good. That's why it's so hard to define!
>
> I'm thinking of something like: if there's $(legitimate) request for changes but the author is dormant for more than $(X) days, then close.
>
> Andrei
>
>
> Macros:
>
> legitimate=?
> X=?

legitimate= consensual, if there is no consensus between phobos devs it cannot be pulled anyway. I'd think those are all changes/PR that are not tagged with »decision block«. I'd think even something like »fix your indentation« is legitimate.
X= 20 days, ask author if he's dead. If he does not respond in 10 days, close and leave a comment to resubmit the pull request on resurrection.

On Friday, 16 January 2015 at 16:22:13 UTC, ketmar via Digitalmars-d wrote:
> it sits in queue without any comments more than 20 days? reject and
> close it.

It is better to have some kind of bot that comment on the PR after a while. Like "hey, this PR is hanging, can someone make thing go forward or I'll close in 2 more month". That generate activity on the PR and is often a wake up call for people.

On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
> I'm thinking of something like: if there's $(legitimate) request for changes but
> the author is dormant for more than $(X) days, then close.

That's also a hamfisted policy. I've seen PR's that were good, but needed a bit of work, but the author disappeared. Sometimes, I've taken those over and finished them.

Arbitrarily closing them means they get lost forever.

On Friday, 16 January 2015 at 18:50:29 UTC, deadalnix wrote:
> On Friday, 16 January 2015 at 16:22:13 UTC, ketmar via Digitalmars-d wrote:
>> it sits in queue without any comments more than 20 days? reject and
>> close it.
>
> It is better to have some kind of bot that comment on the PR after a while. Like "hey, this PR is hanging, can someone make thing go forward or I'll close in 2 more month". That generate activity on the PR and is often a wake up call for people.

On a relevant sidenote: this is a GitHub buildbot that manages Rust's main repo, PR approvals and all that: https://github.com/graydon/bors

IMO that seems to work quite well for Rust and lowers the administrative burden on reviewers.

On 1/16/15 11:23 AM, Walter Bright wrote:
> On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
>> I'm thinking of something like: if there's $(legitimate) request for
>> changes but
>> the author is dormant for more than $(X) days, then close.
>
> That's also a hamfisted policy. I've seen PR's that were good, but
> needed a bit of work, but the author disappeared. Sometimes, I've taken
> those over and finished them.
>
> Arbitrarily closing them means they get lost forever.

Look for a champion after $(X) days? It looks like once a pull request is open it's impossible to close it. There's got to be some garbage collection somehow :o). -- Andrei

On Friday, 16 January 2015 at 19:23:06 UTC, Walter Bright wrote:
> On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
>> I'm thinking of something like: if there's $(legitimate) request for changes but
>> the author is dormant for more than $(X) days, then close.
>
> That's also a hamfisted policy. I've seen PR's that were good, but needed a bit of work, but the author disappeared. Sometimes, I've taken those over and finished them.
>
> Arbitrarily closing them means they get lost forever.

Add an "abandoned" label, and assign it when closing? Then they won't get lost, but also won't clutter the list.

On Fri, 16 Jan 2015 11:23:02 -0800
Walter Bright via Digitalmars-d <digitalmars-d@puremagic.com> wrote:

> On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
> > I'm thinking of something like: if there's $(legitimate) request for changes but
> > the author is dormant for more than $(X) days, then close.
> 
> That's also a hamfisted policy. I've seen PR's that were good, but needed a bit of work, but the author disappeared. Sometimes, I've taken those over and finished them.
> 
> Arbitrarily closing them means they get lost forever.
that's why there will ALWAYS be alot of items in queue and it will not be processed at any sane rate. add ten more people to process the PR queue, and... there will be just more and more unprocessed PRs.

it's cause you're looking at that queue like it's a backpack full of things that can lay there forever. sometimes somebody can pull some thing out of backpack and work on it. but backpack will accumulate more and more things over the time.

this will continue unless you realise that PR queue is not a backpack full of possible cool things. it's a queue of things that *must* *be* *processed* *within* *short* *time*. if it took more than ten days to simply react on the thing -- throw it out, it just cluttering the queue.

this is one of the things that "github culture" gets very-very wrong. a proper place to accumulate requests and code is bugzilla. and PR queue is a queue for things that finally moved to "merging stage". i.e. for things that must be reviewed and merged (or rejected) ASAP.

it doesn't matter how hard somebody will try to keep that queue "manageable", it will be oveflown with requests. the only way to manage it is to turn it from backpack to "ASAP-queue". so i repeat my point: if something sits there for twenty days (let's add ten days) without any motion -- remove it from the queue. either this, or it always be cluttered.

On 1/16/2015 12:44 PM, Andrei Alexandrescu wrote:
> On 1/16/15 11:23 AM, Walter Bright wrote:
>> On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
>>> I'm thinking of something like: if there's $(legitimate) request for
>>> changes but
>>> the author is dormant for more than $(X) days, then close.
>>
>> That's also a hamfisted policy. I've seen PR's that were good, but
>> needed a bit of work, but the author disappeared. Sometimes, I've taken
>> those over and finished them.
>>
>> Arbitrarily closing them means they get lost forever.
>
> Look for a champion after $(X) days? It looks like once a pull request is open
> it's impossible to close it. There's got to be some garbage collection somehow
> :o). -- Andrei

Should we also simply close open Bugzilla issues after X days? I don't think so.

Sometimes we just aren't 'ready' for a particular PR, but it is still valuable, such as the concurrent GC one. Deleting it (what closing really is) is not a solution.

I'd like to revisit the assumption that aged PRs is so bad that they must be removed. It reminds me of how the government got Amtrak trains to run on time by redefining on time from +-5 min to +-30 min.

I do agree that often good PRs get overlooked, and that's shameful. Deleting them is not the answer. Old PRs are not garbage.

On 1/16/2015 12:48 PM, Vladimir Panteleev wrote:
> On Friday, 16 January 2015 at 19:23:06 UTC, Walter Bright wrote:
>> On 1/16/2015 9:49 AM, Andrei Alexandrescu wrote:
>>> I'm thinking of something like: if there's $(legitimate) request for changes but
>>> the author is dormant for more than $(X) days, then close.
>>
>> That's also a hamfisted policy. I've seen PR's that were good, but needed a
>> bit of work, but the author disappeared. Sometimes, I've taken those over and
>> finished them.
>>
>> Arbitrarily closing them means they get lost forever.
>
> Add an "abandoned" label, and assign it when closing? Then they won't get lost,
> but also won't clutter the list.

What should happen is the list of PRs should be sorted on a basis of most-recently-touched-is-first. The older PRs then naturally fade to the 'next' pages. 'clutter' is not a problem, unless we decide that number of open PRs is a proxy for quality of the project.

We should be very careful about working metrics like "number of open PRs". Focus on such can generate an illusion of progress, and actually make things worse.

I've worked at companies that would rate engineers based on the "bug count". That ended very badly, it was so bad it was comical, how working that number actually wrecked the quality of the product. I've seen similar disasters with use of metrics on inventory minimization, and others. Ever since it has made me deeply suspicious about basing quality on such numbers.