Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

> Expecting most teams to be high trust and well functioning is unrealistic.

Maybe we're having two conversations here. Your approach is more correct if the team has low trust or individuals that are difficult to deal with.

My approach is what I see as the ideal to strive for in a team that is cohesive and effective.

Both are worth considering, but, personally, I wouldn't stick around long if I didn't like my team.



Trust is a continuum. After all, the existence of code reviews is merely a formalization of distrust :-)

I think the policies and culture around code review will vary based on the situation (new hire vs seasoned developer, large vs small team size, minor vs major code change, etc). However, I don't think I advocated anything that would be a bad idea even for high trust teams.

The possible exception would be that a code review "not be a request for permission". If you have structured it as a gatekeeping mechanism, simply ensure a "referee" or "tie breaker" to smooth/hasten the process. Even with high trust/functioning teams you'll occasionally get pointless ratholing in the middle of a review, and both the reviewer and the developer should have a way to cross that divide. And no, a "be mature and discuss it through" will not work every time.

As for stating concerns - when is that ever a bad idea? I didn't come up with it, BTW. I learned it from reading books on effective communications. It's a good rule to live for everyday conversation.


I get annoyed when someone ignores my feedback and requests a review from someone else who is known to approve anything. I think doing this is fairly disrespectful and circumvents the point of code review.


Your comment is a good example of what I'm saying:

The problem is not the developer, but the guy who approves everything.

A team that requires code reviews, and has a guy who approves everything, is an unhealthy team.

The developer is asking for a code review not because he wants feedback, but because it is a ritual he has to go through because of team policy. If the team policy changed to "code reviews are for feedback, not permission", then he wouldn't waste your time. He's not the problem - the team's culture/policies is. If the focus was feedback, then you would only get requests from people who want your feedback.

> I get annoyed when someone ignores my feedback

Are you annoyed that he circumvented the purpose of the code review, or that he did not act on your feedback? If the latter, I strongly urge you to change. Acting on feedback should always be optional.

Of course, making code reviews completely optional is a luxury most teams do not have. But one of the reasons they don't have that luxury is that you'll rarely get a high trust team.

Despite this, one can put mitigating protocols with mandatory code reviews to solve some of the problems you're highlighting. If all the code reviews are via Github, and someone else approves the merge when you've left strong feedback suggesting otherwise, the team really needs to have a protocol for handling such scenarios.


I see, I think I'm better understanding what you're getting at, and I think I agree.

> Are you annoyed that he circumvented the purpose of the code review, or that he did not act on your feedback? If the latter, I strongly urge you to change. Acting on feedback should always be optional.

This has only happened to me with a couple of individuals. Usually in these cases I'll leave a comment and it will just never be addressed; the author will request another reviewer and my comment is ignored.

I don't hold PRs hostage and if things go more than one or two rounds of back-and-forth then I'll almost always yield to the author.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: