7 comments

[ 3.2 ms ] story [ 31.0 ms ] thread
I don't really get this. It looks like they jumped on someone who submitted a pull request with a ridiculously harsh "code review", which inevitably provoked a defensive reaction. Way to go discouraging contributions to your project. The funniest thing about this is that the (alleged) code review is so detailed that the time taken to write it could just as easily have been used to actually modify the damn code. This was purely a hazing ritual.

Also, making generalizations about a country with a population of 1.3 billion based on a single interaction is really stupid, and pretty offensive.

> It looks like they jumped on someone who submitted a pull request with a ridiculously harsh "code review", which inevitably provoked a defensive reaction.

It was not harsh. The submitter didn't stick to code guidelines and generally had some issues in his code, which were pointed out. The reviewer's tone may be a little dry, but all the comments were valid and had nothing personal in them. If I was to receive them, I would feel embarrassed of what I did in the code, not because reviewer made fun of my skills or something like that.

I should stress: this was Google Summer of Code work, so the submitter was there clearly to learn something (presumably: from the reviewer). Instead, the response was "oh no, more unnecessary work" and "this code is good".

> The funniest thing about this is that the (alleged) code review is so detailed that the time taken to write it could just as easily have been used to actually modify the damn code. This was purely a hazing ritual.

We could argue about that if it wasn't GSoC. Since it was GSoC, it was supposed to be a learning experience for the submitter. It wasn't a typical, random contribution.

I didn't notice that this was a GSoC project, but I don't really think that makes much difference. If someone submits a bunch of code, and the only kind of comment on it they get is "there's no newline before this 'continue' statement", then that's bound to be frustrating and discouraging. It seems that the developers just didn't want this guy's contributions and were looking for an excuse to reject the code. I think that's certainly the impression that the Indian programmer got, which is what led to his defensive reaction. The whole interaction could have gone very differently if someone had thought to include a simple "thank you" in the comments. As it stands, this is just pointless and counterproductive hazing of a new contributor. It gives the maintainers of the project the chance to grandstand (which they appear to relish), but accomplishes absolutely nothing besides that.
> If someone submits a bunch of code, and the only kind of comment on it they get is "there's no newline before this 'continue' statement",

Oh, far from it. "Newline before `continue'" was raised because project's code guidelines clearly required it. And there were other things, like not using a callback function to .foreach() method for something that could be done in place with `foreach' loop. This was commented as "great, more unnecessary work". Not an attitude of somebody who wants to learn.

Have you actually read the review comments?

> The whole interaction could have gone very differently if someone had thought to include a simple "thank you" in the comments.

Well, the "thank you" comment should clearly be said by the Indian guy. Somebody took the effort to teach him in a setting that from the beginning was exactly this: a person learning from a more experienced one.

> As it stands, this is just pointless and counterproductive hazing of a new contributor.

No. As it stands, it's teaching somebody who came to learn. I remind you again, it's GSoC.

>And there were other things, like not using a callback function to .foreach() method for something that could be done in place with `foreach' loop

I didn't see how that was a big deal. People are always going to disagree somewhat about how to structure code. You can't expect contributors to read your mind and write exactly the code that you would have written. That is why this code review strikes me as a bit disingenuous.

> Well, the "thank you" comment should clearly be said by the Indian guy.

I really don't understand this attitude. Any project should be grateful to people who contribute to it. Personally, I am happy if someone contributes to a project of mine. Their code might not be everything I could have hoped for, but I always thank them for their contribution. If they don't use the same code style as me, I either suck it up and change it myself, or politely ask them if they'd consider resubmitting with the correct style. There is no reason whatever why anyone should be made to feel as if their contributions are unwelcome as a result of such trivial issues. The most basic social graces are sufficient to stop this from happening. And it should be crushingly obvious that it is counterproductive to alienate people who are trying to contribute to your project.

>No. As it stands, it's teaching somebody who came to learn. I remind you again, it's GSoC.

I think that's a misconception. GSoC is a two-way street. Projects get valuable contributions, and the contributors learn from the process. Each party is benefiting and each should be thankful to the other.

Add also the stipend to the landscape. One party gets the code and needs to merge it to the codebase (more work), and the other party learns and is paid for doing so. If the guy is going to be paid, he should do a decent job, including sticking to project's guidelines. Why should he be paid if he only made more work for the project?

> There is no point in giving a line-by-line commentary on how a particular bit of code fails to meet trivial code formatting guidelines.

Well, the reviewer explained the thing in the first encountered line, and then pointed all the lines that need the same treatment so the submitter doesn't have to search for them himself. This is why such comments arise when reviewing somebody's code.

All in all, the reviewer was not rude. He may be thought of as dry, but certainly not rude. The submitter's attitude, on the other hand, showed an attitude, even if it was him that was benefitting from the conversation, not the reviewer. Something with that is off.

The stipend is paid by Google, not by gnome-games. From their perspective they are getting help for free. If they don't want to encourage that, it's their loss. Putting "n00b" contributors down is an easy way to make yourself feel good, but it doesn't ever benefit the project. This kid could have chosen to do 1001 other things beside contribute to gnome-games. Now he's probably wishing that he did. Who benefits?