Thread overview
DMD PR management hits a new low
Nov 18, 2017
Dmitry Olshansky
Nov 18, 2017
Meta
Nov 18, 2017
user1234
Nov 19, 2017
codephantom
Nov 19, 2017
user1234
Nov 18, 2017
Walter Bright
Nov 19, 2017
Iain Buclaw
Nov 19, 2017
Walter Bright
Nov 21, 2017
Iain Buclaw
November 18, 2017
I'll just refer you to this comment: https://github.com/dlang/dmd/pull/6947#issuecomment-345423103

> Manually merging this pull as it sat around long enough waiting to be marked approved that it accumulated github's max 1000 status updates per commit id and won't ever see more until a new commit becomes current for it. Let that sink in... over 1000 builds done for a single pull request before it got marked for merging.

When this happens the auto-tester can no longer update the status of the PR.  There are other PRs also suffering from this problem, just to name two:

https://github.com/dlang/dmd/pull/6435
https://github.com/dlang/dmd/pull/6260

Since the auto-tester can't update its status, it appears as though the auto-tester has stalled with the last status update:
e.g
#6435: auto-tester — Pass: 3, In Progress: 2, Pending: 5
#6220: auto-tester — Pending: 10

I'm not sure what to do about this at this time.  The PRs could be woken up by the original contributor, but submitting a new commit, but it appears some of the contributors have lost patience and moved on.  I'm trying to revive a few, but will my PRs suffer in similar fate?

I'm sorry for being negative, but this is shameful, IMO.  Some of the PRs are very simple, and just need someone with authority to make a decision (e.g. #6435 and #6260)

I'm thinking about creating a monthly list of neglected PRs and submit them to the forum on the 1st of every month to bring some attention to them, but it seems that same information is already available through Github's interface, and I don't know if such an effort would be effective.  But, when the auto-tester can no longer update the PR's status, it make reviewing and making a decision about them, *much* more of a hassle.  We need to prevent PRs from getting to that state.

Can we get some resources allocated to this, please?  What can I do?

Thanks,
Mike

November 18, 2017
On Saturday, 18 November 2017 at 07:52:43 UTC, Michael V. Franklin wrote:
> I'll just refer you to this comment: https://github.com/dlang/dmd/pull/6947#issuecomment-345423103
>
>> Manually merging this pull as it sat around long enough waiting to be marked approved that it accumulated github's max 1000 status updates per commit id and won't ever see more until a new commit becomes current for it. Let that sink in... over 1000 builds done for a single pull request before it got marked for merging.
>

It’s time for us to understand that letting PRs rot in the open and uncertain state is even worse then outrightly rejecting controversial work.

It damages reputation, deters future contributions and clutters the queue.

I’d suggest to put on a grim reaper’s robe and cutdown things that are not attended.
If we were  too eager to close, no worries - just create a new PR.

> Can we get some resources allocated to this, please?  What can I do?
>

Review stuff mostly. To close a ton of PRs we’d need executive decision.

> Thanks,
> Mike


November 18, 2017
On Saturday, 18 November 2017 at 13:08:55 UTC, Dmitry Olshansky wrote:
> On Saturday, 18 November 2017 at 07:52:43 UTC, Michael V. Franklin wrote:
>> I'll just refer you to this comment: https://github.com/dlang/dmd/pull/6947#issuecomment-345423103
>>
>>> Manually merging this pull as it sat around long enough waiting to be marked approved that it accumulated github's max 1000 status updates per commit id and won't ever see more until a new commit becomes current for it. Let that sink in... over 1000 builds done for a single pull request before it got marked for merging.
>>
>
> It’s time for us to understand that letting PRs rot in the open and uncertain state is even worse then outrightly rejecting controversial work.
>
> It damages reputation, deters future contributions and clutters the queue.
>
> I’d suggest to put on a grim reaper’s robe and cutdown things that are not attended.
> If we were  too eager to close, no worries - just create a new PR.

Agreed. I tried my hand at this awhile ago with the Phobos queue and went on a mini closing/merging/pinging spree, but was very conservative at the risk of stepping on toes and thus was only able to get the number of open PRs down to 90 (and now it's back up over 110 again). I've noticed, however, that while Phobos/Druntime get most of the attention, dmd is really languishing. The are currently 182(!) open pull requests for dmd, with over half (99) being open for 1 year or more. The situation is getting out of control.

https://github.com/dlang/dmd/pulls?utf8=✓&q=is%3Apr%20is%3Aopen%20created%3A<2016-11-18

>> Can we get some resources allocated to this, please?  What can I do?
>>
>
> Review stuff mostly. To close a ton of PRs we’d need executive decision.
>
>> Thanks,
>> Mike


November 18, 2017
On Saturday, 18 November 2017 at 07:52:43 UTC, Michael V. Franklin wrote:
> I'll just refer you to this comment: https://github.com/dlang/dmd/pull/6947#issuecomment-345423103
>
>> Manually merging this pull as it sat around long enough waiting to be marked approved that it accumulated github's max 1000 status updates per commit id and won't ever see more until a new commit becomes current for it. Let that sink in... over 1000 builds done for a single pull request before it got marked for merging.
>
> When this happens the auto-tester can no longer update the status of the PR.  There are other PRs also suffering from this problem, just to name two:
>
> https://github.com/dlang/dmd/pull/6435
> https://github.com/dlang/dmd/pull/6260
>
> Since the auto-tester can't update its status, it appears as though the auto-tester has stalled with the last status update:
> e.g
> #6435: auto-tester — Pass: 3, In Progress: 2, Pending: 5
> #6220: auto-tester — Pending: 10
>
> I'm not sure what to do about this at this time.  The PRs could be woken up by the original contributor, but submitting a new commit, but it appears some of the contributors have lost patience and moved on.  I'm trying to revive a few, but will my PRs suffer in similar fate?
>
> I'm sorry for being negative, but this is shameful, IMO.  Some of the PRs are very simple, and just need someone with authority to make a decision (e.g. #6435 and #6260)
>
> I'm thinking about creating a monthly list of neglected PRs and submit them to the forum on the 1st of every month to bring some attention to them, but it seems that same information is already available through Github's interface, and I don't know if such an effort would be effective.  But, when the auto-tester can no longer update the PR's status, it make reviewing and making a decision about them, *much* more of a hassle.  We need to prevent PRs from getting to that state.
>
> Can we get some resources allocated to this, please?  What can I do?
>
> Thanks,
> Mike

For comparison:

- https://github.com/rust-lang/rust/pulls?page=1&q=is%3Apr+is%3Aopen+sort%3Aupdated-asc: least recent update for a PR was 8 days old
- https://github.com/dlang/dmd/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc: least recent update for a PR was 2 years and 7 months old, and it's a "pre-boostrap" one.

Now look at this:

https://github.com/rust-lang/rust/pull/45244#issuecomment-345455131
https://github.com/rust-lang/rust/pull/45501#issuecomment-343992636
https://github.com/rust-lang/rust/pull/45558#issuecomment-343497687

It seems that they close systematically after a month or so.
You need to hire a vilain that will do this dirty job for you (or maybe even use a bot)...everybody will hate him but the stack of folder on the desktop will get smaller.
November 18, 2017
On 11/17/2017 11:52 PM, Michael V. Franklin wrote:
> What can I do?

Doing what you just did is the right thing. The forum is for topical things like bringing attention to neglected items.
November 19, 2017
On Saturday, 18 November 2017 at 23:21:08 UTC, user1234 wrote:
> It seems that they close systematically after a month or so.
> You need to hire a vilain that will do this dirty job for you (or maybe even use a bot)...everybody will hate him but the stack of folder on the desktop will get smaller.

Sounds like a job for SysAdmin.

A smart sysadmin would implement the bot option (cause then you can blame the bot).

Just need to establish clear and fair criteria for the bot, and then implement it.

It's the fairest way to proceed, and it's automated.

People can then argue with the criteria, and leave sysadmin alone.

And if you think you have a good pull request, and it keeps getting closed, then you have to do more work to get people's attention (the right people). That's all there is to it.

"Although it's never nice to reject someone's work it's preferable to leaving pull requests open that you will never merge. Those pull requests will just hang over you and the contributor indefinitely. One of the indicators of a healthy project is its responsiveness to contributions, whether it is giving feedback, merging, or closing pull requests."

https://github.com/blog/2124-kindly-closing-pull-requests

November 19, 2017
On Sunday, 19 November 2017 at 01:40:32 UTC, codephantom wrote:
> On Saturday, 18 November 2017 at 23:21:08 UTC, user1234 wrote:
>> It seems that they close systematically after a month or so.
>> You need to hire a vilain that will do this dirty job for you (or maybe even use a bot)...everybody will hate him but the stack of folder on the desktop will get smaller.
>
> Sounds like a job for SysAdmin.
>
> A smart sysadmin would implement the bot option (cause then you can blame the bot).
>
> Just need to establish clear and fair criteria for the bot, and then implement it.

I'd propose these rules for the dbot:

- no human review comment (codecov + dbot automatically comment): never close
- never close if tag is set (e.g DIP implementation, experimental phobos addition, etc.)
- three months after last update: warn.
- six months after last update: close, politely, w/ explanations

Personally i don't understand how people can stand to have their old PRs opened.
Usually i close myself after 1 month, which is rare, usually things are handled in the two weeks that follow the opening. I say that but i've never submitted anything big...



November 19, 2017
On 19 November 2017 at 00:44, Walter Bright via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> On 11/17/2017 11:52 PM, Michael V. Franklin wrote:
>>
>> What can I do?
>
>
> Doing what you just did is the right thing. The forum is for topical things like bringing attention to neglected items.

When I have time, I have it in my todo list to review everything below PR 6000.

https://issues.dlang.org/show_bug.cgi?id=17839
November 19, 2017
On 11/19/2017 3:54 AM, Iain Buclaw wrote:
> When I have time, I have it in my todo list to review everything below PR 6000.
> 
> https://issues.dlang.org/show_bug.cgi?id=17839

Thank you, Iain! Recently I did a look through the old ones for optlink bugs, and found a number of them were already fixed, duplicates or no longer relevant.
November 21, 2017
On 20 November 2017 at 00:11, Walter Bright via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> On 11/19/2017 3:54 AM, Iain Buclaw wrote:
>>
>> When I have time, I have it in my todo list to review everything below PR 6000.
>>
>> https://issues.dlang.org/show_bug.cgi?id=17839
>
>
> Thank you, Iain! Recently I did a look through the old ones for optlink bugs, and found a number of them were already fixed, duplicates or no longer relevant.


FYI, this will likely be over the holiday period.  Probably the most high priority ones to address are all Kenji's old PRs.