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

Jonathan M Davis jmdavisProg at gmx.com
Mon May 2 13:45:22 PDT 2011


> Guidelines for this would be nice to have.
> 
> I'd say definite changes that should go through a reviewed pull request:
> 
> * changes from non-phobos developers (obviously)
> * changes to files that you do not "own" (even if trivial).  For example,
> if I were to find a bug in std.parallelism, I shouldn't commit, I should
> generate a pull request for review.
> 
> * changes that alter the design of a module.
> * new code (including adding classes/structs/functions to an existing
> module)
> 
> 
> I think everything else is OK to commit directly instead of generating a
> pull request, including bug fixes (as long as they are not fixed by doing
> one of the above).
> 
> 
> What do you think?  A list like this (and actually, our entire process
> should be documented, including how to get included as a phobos developer)
> should be posted to the web site.

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.

My one real concern with pull requests though is that not enough people have 
been paying attention to them. On the whole, a fairly number of Phobos 
developers have been commenting on pull requests with any regularity, and it 
becomes more burdensome when there are very few people looking at pull 
requests. It also slows the process down more than it might otherwise.

Though I get the impression that the general level of activity of Phobos 
developers has dropped off since the switch the git and github. There's still 
definitely work being done, and I'd have to check the archives to be sure, but 
I'm pretty sure that several developers who were at least semi-active before 
have essentially ceased checking in code changes. I don't know if it has 
anything to do with the source control switch or not, but I definitely get the 
impression that fewer Phobos developers have been active, which makes it 
harder to work through pull requests, since there are that many fewer people 
reviewing them.

In any case, Lars created a nascent guide for contributions to Phobos, which 
we should probably build on and update as necessary:

http://www.kyllingen.net/guide.html

So, if you're looking for documentation, that's what we have. Once we're 
certain that it's what we want, we should probably post it on d-programming-
language.org and/or www.digitalmars.com.

- Jonathan M Davis


More information about the phobos mailing list