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

Jonathan M Davis jmdavisProg at gmx.com
Mon May 2 14:27:55 PDT 2011


> > On Mon, May 2, 2011 at 4:45 PM, Jonathan M Davis 
<jmdavisProg at gmx.com>wrote:
> > > I'm a bit divided on it. Andrei and Lars were pushing for _all_
> > > check-ins to
> > > be reviewed. Part of me thinks that that's overkill and yet there have
> > > been times that other developers have caught stuff that I likely
> > > wouldn't have even
> > > with smaller changes.
> > > 
> > > It does seem like overkill to require a pull request for smaller
> > > changes, particularly if they really don't look like they'd cause a
> > > problem, but at the
> > > same time, the extra eyes can be really valuable, even when you don't
> > > expect
> > > it.
> > > 
> > > The truly minor, non-code stuff - such as updating the changelog -
> > > shouldn't
> > > need a pull request, but at this point, I'm inclined to agree with
> > > Andrei and
> > > Lars that all (or at least very nearly all) code changes should go
> > > through pull requests.
> > 
> > Actually, one of my big concerns with using pull requests is that I want
> > near-instant feedback from the auto tester to make sure my stuff works on
> > all platforms.  It's frustrating to have to wait an indeterminate amount
> > of time for such feedback.
> 
> I definitely understand that, but if we do that, then code isn't getting
> reviewed. And the reviews on github have definitely been helping improve
> code. As far as code quality goes, pull requests are very much a good
> thing.
> 
> The flip-side, of course, is the delays. It's not uncommon for a pull
> request to sit there for a week or more (in fact, I believe that a pull
> request that gets pulled in in less than a week would be a rarity at this
> point). But unless we have more people who are regularly reviewing and
> commenting on pull requests, I don't know how to fix that.
> 
> Ideally, there would be a way to test your code on all platforms _before_
> creating a pull request for it, but at the moment, short of setting up all
> of the systems yourself for yourself, there's no way to do that.
> 
> 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. The
> question is how we deal with and fix the problems that arise from them
> (particularly the delays), and whether _all_ code changes should go
> through pull requests. And as I said, I lean towards requiring that
> everything but the smallest of changes go through pull requests, but I
> agree that the current delays in that process are definitely problematic.

I do think that that it's worth noting, however, that the sort of stuff that 
you've been working on lately, David, (i.e. std.parallelism) is likely much 
more dependent on the platform that it's running on than most of Phobos. Most 
of Phobos is quite platform independent, in which case, platform-specific bugs 
tend to be bugs in the compiler. So, that does tend to change your perspective 
and needs a bit. There are other modules (such as std.datetime) which are 
definitely affected by the platform that they're on, but most aren't. So, in 
the general case, the need to test code on multiple platforms shouldn't be 
anywhere near as critical as what you've been experiencing.

- Jonathan M Davis


More information about the phobos mailing list