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?

On Saturday, July 27, 2013 12:35:40 monarch_dodra wrote:
> On Monday, 22 July 2013 at 18:08:36 UTC, Andrei Alexandrescu
> 
> wrote:
> > Please join me in congratulating monarch dodra for his admission among our github committers. We're starting with phobos, druntime, and tools access, and if all goes well, we'll extend write rights to dmd also.
> > 
> > 
> > Thanks,
> > 
> > Andrei
> 
> Hey! Sorry it took me 4 days to discover the D announce thread. I must admit it wasn't one of the threads I looked over (although this will change starting today, lots of interesting articles here).
> 
> In any case, thank you everyone for your support and kind words :)
> 
> I don't think granting me dmd rights would be necessary, as I don't really know how the compiler works, and I mostly only contribute to phobos anyways (sometimes druntime).
> 
> I may in the future try to contribute to dmd itself, but I wouldn't ever pull anything in there myself anyways, so giving me pull rights (at this point) would be mostly pointless.
> 
> Well, here's to making D as best as it can be ^^

I wouldn't want dmd commit rights for the same reason. I tend to think that commit rights to dmd and those to the standard library should be separate things, as there's quite a difference between what kind of understanding is required to work on Phobos from what is required to work on dmd (though in most cases, someone with commit rights to dmd should probably have commit rights to the libraries).

In any case, welcome aboard!

- Jonathan M Davis

On Saturday, 27 July 2013 at 11:14:06 UTC, Joseph Rushton Wakeling wrote:

> 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."

Github supports tags. I don't know if it's possible to use tags when issues are disabled though.

--
/Jacob Carlborg