www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DMD PR management hits a new low

reply Michael V. Franklin <slavo5150 yahoo.com> writes:
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
Nov 17
next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
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
Nov 18
parent Meta <jared771 gmail.com> writes:
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
Nov 18
prev sibling next sibling parent reply user1234 <user1234 12.nl> writes:
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.
Nov 18
parent reply codephantom <me noyb.com> writes:
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
Nov 18
parent user1234 <user1234 12.nl> writes:
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...
Nov 18
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
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.
Nov 18
parent reply Iain Buclaw <ibuclaw gdcproject.org> writes:
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
Nov 19
parent reply Walter Bright <newshound2 digitalmars.com> writes:
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.
Nov 19
parent Iain Buclaw <ibuclaw gdcproject.org> writes:
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.
Nov 21