On Wed, 08 May 2013 10:56:28 -0400, Benjamin Thaut
<c...@benjamin-thaut.de> wrote:
I recently got very disappointed by the review process of a pull request
I did on druntime:
https://github.com/D-Programming-Language/druntime/pull/370
This is how it went:
1) First everyone nitpicked about code formatting
2) I fixed all the nitpicks which was quite some work.
3) Pull request stalls for a month.
4) Even more nitpicks.
5) Even more work.
6) Reviewers actually start thinking about the problem behind the pull
request.
7) Problem is not important enough, review request gets rejected.
8) All the work, including fixing nitpicks is wasted.
So what I'm trying to say is, that maybe a pull request should first be
analyzed if it is actually worth putting more work into it before
starting with the nitpicks. I don't know if the review process is
already defined somewhere, if not it might be worth doing so.
I first of all want to say, good for you to try helping the D community.
This is something we need a lot of. I also want to say that I am a
contributor to both Phobos and Druntime, and I also have been largely
unable to look at pull requests, shame on me. This is super-frustrating
for authors, and it's going to kill contributions if we don't fix it.
But I want to give a little bit more of an explanation here. I was not
involved in the pull request. But I can see the history. Some
observations I have:
1. nitpicking of style is EASY, it requires no thought or real time,
except to type the nit. This makes it something more likely to occur
before people have put any real thought into a pull request. The lesson
here should be to use the correct style so you don't have this problem.
I'm aware that we don't have a good definition, or good consistency, and
we need to do both of those.
2. The people who complained about style/formatting were NOT the same
people who questioned the utility of the pull request. This is HUGELY
important in understanding how this all went down.
For point 2, realize that people have their own schedules and own day-time
jobs. I literally ignored the newsgroups for 6 months because I did not
have time to look at D at all. From the point of view of the requester,
it is really important to understand that there is no real order to pull
requests to review. It's not like people can't review more than one pull
request, have to review them in a specific order, or have to review them
at all. And the goals and motivations for review can be different. We
don't all speak as one voice, so try to remember that.
This is something I think can be better managed with a collaboration tool
(like trello), or at least a status notification of who is assigned, who
is reviewing, what status is it in, etc. There is a large nebulous thing
called a review process that is right now defined loosely (AFAIK, the only
requirement is we have 2 people review each request), and pretty much
differs for every person. This needs to change. We need an official
review process, and a well-defined order of how things should be done.
Otherwise, the requester has no clue what is happening, and even other
reviewers have no idea what is happening. I will start a new thread on
this.
Sorry to read about this experience. I hope we can all do better.
-Steve
