On 5/3/11 12:38 AM, Don Clugston wrote:
On 2 May 2011 23:18, Jonathan M Davis<[email protected]> 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
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos
