[phobos] [D-Programming-Language/phobos] a15e68: Accidentally committed with a bunch of debugging c...

[Andrei Alexandrescu]

On 5/3/11 12:38 AM, Don Clugston wrote:
> On 2 May 2011 23:18, Jonathan M Davis<jmdavisProg at gmx.com>  wrote:
>> Still, on the whole, I think that the pull requests have had a positive
>> influence on code quality, and I think that they're a good thing.
>
> I have to say, I *totally* disagree with that. We've just three
> completely erroneous pull requests merges in.
> The autotester is currently failing because of it.
> (1) your changes to std.datetime, which shouldn't have been pulled in
> until the CTFE bug was fixed;
> (2) two pull requests moving std.math intrinsics to a new druntime
> file core.math.
> This had been discussed as being as being a bad idea, and yet it got
> pulled in anyway, and it doesn't even compile Phobos, on ANY platform.
>
> I would say, the evidence shows:
> (1)  the pull request system DEFINITELY slows development down dramatically.
> (2) the pull request system DOES NOT improve quality.
>
> Obviously, code reviews are good.
> The pull requests seem to be working well for DMD. But for Phobos,
> basically, they suck.

I think that would be too harsh an assessment. Let's not be hasty to 
generalize.

First, the author of a pull request is to be trusted that has tested the 
code on at least one platform - no ifs and buts. That's what allows the 
person approving a pull request to click "automatic merge" after 
reviewing for potential platform issues. In case of platform-sensitive 
code, the pull request author must request testing on other platforms. 
(I do realize that such a decision has fuzzy boundaries.)

Second, in the (hopefully) rare cases when the trunk does break, it's 
not difficult to turn the clock back to a working version. That makes 
the slowdown, when it does occur, nowhere near dramatic.

The pull request system is here to stay. It allows wide collaboration, 
makes for a palpable sense of achievement for contributors, and fosters 
an atmosphere of collaboration and excellence. What we need to do is 
improve our tools, process, and git mastery.


Andrei