Jump to page: 1 2 3
Thread overview
The Phantom Zone
Jan 15, 2018
Walter Bright
Jan 15, 2018
Mike Franklin
Jan 16, 2018
Martin Nowak
Jan 16, 2018
Brad Roberts
Jan 16, 2018
Walter Bright
Jan 16, 2018
Brad Roberts
Jan 16, 2018
Walter Bright
Jan 17, 2018
Brad Roberts
Jan 17, 2018
Walter Bright
Jan 18, 2018
Rubn
Jan 17, 2018
Paolo Invernizzi
Jan 17, 2018
Paolo Invernizzi
Jan 17, 2018
Martin Nowak
Jan 16, 2018
qznc
Jan 16, 2018
Sebastian Wilzbach
Jan 17, 2018
Martin Nowak
Jan 16, 2018
Walter Bright
Jan 16, 2018
John Gabriele
Jan 16, 2018
Andrew Benton
Jan 17, 2018
Martin Nowak
Feb 07, 2018
Iain Buclaw
Feb 07, 2018
Iain Buclaw
January 14, 2018
The issue that there are too many aged PRs on github that sit around for years comes up again and again. I've resisted just closing them, because closing them is tantamount to termination with prejudice:

https://en.wikipedia.org/wiki/Termination_of_employment#Rehire_following_termination

and the PR is never seen again. This is squandering our resources. A PR may remain open, but is not pulled, for several reasons:

1. it needs work, but the author has left the field
2. it's a good idea whose time has not yet come
3. it's a good idea, but a not good enough implementation, but the PR still contains valuable insight, details, and test cases that would be useful for a more acceptable implementation
4. it's large and complex and nobody has been willing to expend the effort to do a proper review.

But github has a feature we can use for this. We tag these PRs with "Phantom Zone" and then close them.

http://superman.wikia.com/wiki/The_Phantom_Zone
(People with terminal diseases would get put in the Phantom Zone until a cure could be found.)

They can then be viewed via this link:

https://github.com/dlang/dmd/pulls?q=is%3Apr+is%3Aclosed+label%3A%15Phantom+Zone%15

PRs banished to the Phantom Zone would have to have an explanatory comment saying why.

Andrei has suggested calling it "Limbo".

January 15, 2018
On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
> The issue that there are too many aged PRs on github that sit around for years comes up again and again. I've resisted just closing them, because closing them is tantamount to termination with prejudice:
>
> https://en.wikipedia.org/wiki/Termination_of_employment#Rehire_following_termination
>
> and the PR is never seen again. This is squandering our resources.

In general, I agree.

> A PR may remain open, but is not pulled, for several reasons:
> 1. it needs work, but the author has left the field

If the PR has potential, we should invest in it; not close it.  I and others have done this recently by adopting PRs and shepherding them through the process.  IMO this has been quite successful.

> 2. it's a good idea whose time has not yet come

In this case, I suggest documenting the idea in Bugzilla with a link to the PR, then close the PR.  I believe the PR will still remain in GitHub's history so it can be revisited when its time comes.

> 3. it's a good idea, but a not good enough implementation, but the PR still contains valuable insight, details, and test cases that would be useful for a more acceptable implementation

In this case, we should be investing in the contributor.  Help the contributor get it right.  Not only will we get a better contribution out of the process, but we'll have gained a better contributor that could potentially become a profitable asset.

> 4. it's large and complex and nobody has been willing to expend the effort to do a proper review.

Help the contributor break it up.

* Give them advice on how they can make it easier to review.
* Create a roadmap of reviewable steps that the contributor can follow to see the implementation through to completion, and reviewers can refer to to understand what role each submission plays in the larger goal.

> But github has a feature we can use for this. We tag these PRs with "Phantom Zone" and then close them.
>

I need to think about this some more, but at the moment, the idea doesn't appeal to me.

I would like for us, before we submit a new contribution, look at the existing contributions and ask "Is it possible for me to invest in one those existing contributions before submitting a new one?".

Sometimes, however, a new contribution is exactly what the existing PRs need.  For example fixing this bug (https://issues.dlang.org/show_bug.cgi?id=18191) would help me revive https://github.com/dlang/dmd/pull/7388  Iain recently mentioned on slack that he has a couple more like this that are currently in the PR queue.

But, you get the idea:  We should be doing more to remove obstacles for others before adding more work to the queue.

All that being said, we also need to remove obstacles that are preventing a PR from being closed.  If a PR is just not a good idea, or the investment to get the implementation into shape would be too great, we should do our best to close it quickly and diplomatically, so it doesn't compete for resources with more high-value contributions.  This might include:

1.  Politely rejecting the idea with justification
2.  Suggesting a potential alternative
3.  Thoroughly documenting the idea in Bugzilla and suggesting it be revisited at a later time
4.  etc...

Mike


January 16, 2018
On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
> But github has a feature we can use for this. We tag these PRs with "Phantom Zone" and then close them.
>
> http://superman.wikia.com/wiki/The_Phantom_Zone
> (People with terminal diseases would get put in the Phantom Zone until a cure could be found.)
>
> They can then be viewed via this link:
>
> https://github.com/dlang/dmd/pulls?q=is%3Apr+is%3Aclosed+label%3A%15Phantom+Zone%15
>
> PRs banished to the Phantom Zone would have to have an explanatory comment saying why.
>
> Andrei has suggested calling it "Limbo".

What other projects are occasionally doing is a mass exodus of all (stale) PRs. Asking people (via comment) to create new ones (hurdle) if they still care enough.
I've seen this many times, sometimes because repos are migrated (good reason), sometimes just because it's too many.

I'd suggest that someone plans a friendly spring-cleaning with a helpful message. It won't be very disturbing IMO.
Making sure to get all the right stalled PRs is another task. Maybe just take everything > 12 month or so. Some research into PR stats would be helpful, e.g. likelihood of merge after X days, to inform a good value.

Tagging closed PRs as phantom zone is a good idea.
Let's just no do it continuously, but as properly communicated mass cleaning.

Overall this sounds like a couple of bash lines, candidate for https://github.com/MartinNowak/github_scripts.
January 15, 2018
On 1/15/2018 7:50 PM, Martin Nowak via Dlang-internal wrote:
> On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
>> But github has a feature we can use for this. We tag these PRs with "Phantom Zone" and then close them.
>>
>> http://superman.wikia.com/wiki/The_Phantom_Zone
>> (People with terminal diseases would get put in the Phantom Zone until a cure could be found.)
>>
>> They can then be viewed via this link:
>>
>> https://github.com/dlang/dmd/pulls?q=is%3Apr+is%3Aclosed+label%3A%15Phantom+Zone%15 
>>
>>
>> PRs banished to the Phantom Zone would have to have an explanatory comment saying why.
>>
>> Andrei has suggested calling it "Limbo".
>
> What other projects are occasionally doing is a mass exodus of all (stale) PRs. Asking people (via comment) to create new ones (hurdle) if they still care enough.
> I've seen this many times, sometimes because repos are migrated (good reason), sometimes just because it's too many.
>
> I'd suggest that someone plans a friendly spring-cleaning with a helpful message. It won't be very disturbing IMO.
> Making sure to get all the right stalled PRs is another task. Maybe just take everything > 12 month or so. Some research into PR stats would be helpful, e.g. likelihood of merge after X days, to inform a good value.
>
> Tagging closed PRs as phantom zone is a good idea.
> Let's just no do it continuously, but as properly communicated mass cleaning.
>
> Overall this sounds like a couple of bash lines, candidate for https://github.com/MartinNowak/github_scripts.

What I'd like to see as a first step is to draw a line in the sand, every new pull request as of 1/1/2018 should be handled to resolution, period.  Don't let them get stale.  If really necessary, maybe taking the PZ action, but that'd really be more of a failure than success, but less so than just letting it age into the lower depths of the queue.

Hopefully that first step can be taken and successfully pulled off. If so, then some effort can be put towards shrinking the backlog. But I haven't seen any real evidence out side of short flurry of activity that we can even keep up.
January 16, 2018
On 1/15/2018 8:41 PM, Brad Roberts wrote:
> What I'd like to see as a first step is to draw a line in the sand, every new pull request as of 1/1/2018 should be handled to resolution, period.  Don't let them get stale.

We've tried that again and again. It presupposes that all PRs have a resolution. They do not - I mentioned 4 common reasons why in the opening post.

There is another problem here. I just got back from POPL 2018 with a list of ideas I want to get going on. I have made zero progress on any of them, because I spend all the time reviewing other peoples' work, dealing with can you please look at these bug reports, and one entire evening rebasing PRs due to the torrent of refactorings.

You're asking me to redouble my efforts at looking at PRs. It's just not possible.
January 16, 2018
On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
> PRs banished to the Phantom Zone would have to have an explanatory comment saying why.

I would suggest an alternative approach: Automate it.

1. When there is no activity on a PR for one month, the bot pings the author and asks for the status.

2. When there is no activity on a PR for four months, the bot pings three random other people and asks them to adopt the PR. Maybe it can be more clever then "random" and look at what files are touched or something.

3. When there is no activity on a PR for half a year, the bot leaves a short comment "no activity for half a year." and closes the PR.

If someone wants to keep a PR alive it only takes single comment every six months.

I doubt that there is any PR which got zero activity for six months, yet was successfully merged later.
January 16, 2018

On 2018-01-16 19:32, qznc via Dlang-internal wrote:
> On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
>> PRs banished to the Phantom Zone would have to have an explanatory comment saying why.
> 
> I would suggest an alternative approach: Automate it.
> 
> 1. When there is no activity on a PR for one month, the bot pings the
> author and asks for the status.
> 2. When there is no activity on a PR for four months, the bot pings
> three random other people and asks them to adopt the PR. Maybe it can
> be more clever then "random" and look at what files are touched or
> something.

The bot currently marks a PR has stalled if there hasn't been any activity within three months

> 3. When there is no activity on a PR for half a year, the bot leaves a
> short comment "no activity for half a year." and closes the PR.

This would be trivial to implement, but I haven't done this so far, because there hasn't been a consensus on this.

> If someone wants to keep a PR alive it only takes single comment every
> six months.
> 
> I doubt that there is any PR which got zero activity for six months,
> yet was successfully merged later.

Hehe, it does happen sometimes, e.g.

https://github.com/dlang/dmd/pull/4988

Also Rainers recently rebased #2480:

https://github.com/dlang/dmd/pull/2480

But obviously not too often.
January 16, 2018
On 1/16/2018 3:13 AM, Walter Bright via Dlang-internal wrote:
> On 1/15/2018 8:41 PM, Brad Roberts wrote:
>> What I'd like to see as a first step is to draw a line in the sand, every new pull request as of 1/1/2018 should be handled to resolution, period.  Don't let them get stale.
> 
> We've tried that again and again. It presupposes that all PRs have a resolution. They do not - I mentioned 4 common reasons why in the opening post.

They do, since closing with some tagging as a way to find them later is one of the possible resolutions, just as a last resort.

> There is another problem here. I just got back from POPL 2018 with a list of ideas I want to get going on. I have made zero progress on any of them, because I spend all the time reviewing other peoples' work, dealing with can you please look at these bug reports, and one entire evening rebasing PRs due to the torrent of refactorings.

Yup, pretty standard life of a senior developer.  I'm sure you're used to it.

> You're asking me to redouble my efforts at looking at PRs. It's just not possible.

This isn't about you but rather the entire group of D developers.  I'm not sure how to effect the change, but imho, keeping the pull request queue clean needs to be higher priority than producing additional pulls.  It's not that the bug fixing, code cleanup, new features, etc aren't important, but that they're one step behind making sure existing hard work is appreciated and followed through on.  Every single place I've ever worked getting code reviewed and checked in has taken precedence over writing new code.

----

I guess I should ask a question:  What's your goal with the PZ?  Is it to have the github pull queues kept to a tiny number of open and active requests?

Great, that's exactly what I just suggested with the primary difference being just focusing on the ones from the start of the year as a trial run before applying the same policies to the rest of the queue.

If not, then what is the goal and what's the differences that you envision?
January 16, 2018
On 1/16/2018 12:35 PM, Brad Roberts wrote:
> What's your goal with the PZ?

The current system is a binary "it's good enough to merge now" and "it has zero merit whatsoever and should never be looked at again."

There's a continuum in between those poles, and that is what the PZ is for. I listed 4 reasons a PR may fall in that continuum.

> Is it to have the github pull queues kept to a tiny number of open and active requests?

One is to keep the autotester from wasting resources on those.

The other is the political/marketing issue of "what are those old PRs doing there? They should just be merged or closed!"
January 16, 2018
On 1/16/2018 10:32 AM, qznc wrote:
> On Monday, 15 January 2018 at 01:10:58 UTC, Walter Bright wrote:
>> PRs banished to the Phantom Zone would have to have an explanatory comment saying why.
> 
> I would suggest an alternative approach: Automate it.

Banishment to the PZ requires judgement.
« First   ‹ Prev
1 2 3