monarch dodra granted write access to phobos, druntime, and tools

Joseph Rushton Wakeling joseph.wakeling at webdrake.net
Sat Jul 27 04:14:05 PDT 2013


On Tuesday, 23 July 2013 at 19:24:10 UTC, Andrei Alexandrescu 
wrote:
> I'm very surprised by your outlook. My perception is that the 
> long queue of pending pull requests not being reviewed is the 
> single most important bottleneck at this point in history in 
> the path of D. By my estimates I think we'd improve the speed 
> of D's development by at least one third if we solve this one 
> issue. There's no other issue offering so much impact.

I agree it's the major bottleneck but disagree slightly about the 
details.

My recent experience has been that my Phobos pull requests get 
_reviewed_ quite quickly but then it may take quite some time to 
actually get merged. Confusion can be added because the reviewers 
don't always indicate explicit approval of the code, so the 
submitter can be sitting in limbo not knowing if the lack of 
merge is down to the code still being inadequate or just the 
reviewer not getting round to merging it yet.

The latter kind of delay tends to result from the situation where 
the reviewer is waiting for the test suite to pass. Because 
there's no option to auto-merge on pass, and no alert to 
reviewers that a pull request has passed testing, it's easy to 
miss windows of opportunity to merge. This only has to happen a 
few times for the pull request to get stuck at the bottom of the 
test queue and for the delay in merging just to stretch.

So, I'd propose that if possible the review process include a way 
for reviewers to explicitly indicate, "This pull request is 
provisionally approved subject to testing."

Approved pull requests would go on a separate priority test queue 
with "first in, first out" policy. If the test suite passes, 
they're auto-merged, if it fails they are removed from the queue 
and must be re-approved.

Ideally it should be possible to distinguish actual test failures 
from something going wrong with the test procedure itself (e.g. a 
test process not spawning correctly) and in the latter case 
keeping the pull request in the approved queue.

Does this sound workable/useful?


More information about the Digitalmars-d-announce mailing list