Reviewing pull requests

Steven Schveighoffer schveiguy at yahoo.com
Mon Mar 24 14:26:30 PDT 2014


On Mon, 24 Mar 2014 16:53:36 -0400, Brad Roberts <braddr at puremagic.com>  
wrote:

> On 3/24/14, 1:48 PM, Steven Schveighoffer wrote:
>> On Mon, 24 Mar 2014 16:11:34 -0400, Andrej Mitrovic  
>> <andrej.mitrovich at gmail.com> wrote:
>>
>>> On Monday, 24 March 2014 at 20:02:51 UTC, Andrei Alexandrescu wrote:
>>>> The single most impactful way to improve D some more at this point  
>>>> is, without a shred of doubt,
>>>> reviewing pull requests on github.
>>>
>>> I've been through most Phobos pulls several times now in the last few  
>>> months, and from what I can
>>> tell a lot of pulls seem to be stalled by the pull authors themselves  
>>> rather than a lack of
>>> reviewers. Someone makes a pull, it gets reviewed but turns out the  
>>> pull needs more work, and then
>>> the author disappears from the face of the earth.
>>
>> 2 things:
>>
>> 1. Pulls that are waiting for author changes, but haven't been touched  
>> in a week (maybe?) should be
>> closed. They can always be reopened.
>> 2. Pulls that are closed do not get tested, so they are not using up  
>> cycles on the auto tester.
>
> Older than 2 weeks and they aren't likely to get tested either.. since  
> any change to the branch (ie, a pull being merged) restarts testing, so  
> they're effectively just dead weight down the queue.  Only a lull in  
> pull merging would allow them to be tested, which is fine since a lull  
> in activity would otherwise just result in idle testers.  So, really #2  
> is a non-issue.

OK, so it tests in reverse chronological? That makes the most sense. I was  
worried it was taking away time from other test runs.

If a test has been started, and a merge occurs, does it stop the test?

-Steve


More information about the Digitalmars-d mailing list