Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Confessions of a programmer: I hate code review (2010)

blog.nelhage.com

Post reply on HN
Print view View original

71–80 of 165 posts

Re: Confessions of a programmer: I hate code review (2010)

#71

Post by lmm »

> If I spend a day doing nothing but reading code reviews, I'll end up feeling unsatisfied and unproductive. Because code review feels fundamentally optional -- even though I believe it's beneficial, it's something we've chosen to do, not something that we absolutely have to do in order for the project or business to keep operating -- it's more frustrating to find myself spending a large amount of time on.

This is where you need to change your mindset from writing code to delivering business value. At my first job we realized that tickets were actually spending longer in code review than in development - and therefore our primary bottleneck on delivering functionality was code review. So as a developer code review is actually your main job, and writing code is something you do in your downtime when you don't have any reviews available.

Once everyone in the team is making reviewing code their top priority (or at least, a higher priority than writing their own code), waiting for review becomes much less of a problem.

Re: Confessions of a programmer: I hate code review (2010)

#72

Post by bryanrasmussen »

RandomInteger4 wrote: post #58
bryanrasmussen wrote: post #39

Earlier quoted context omitted.

so another story here: code review mandatory at last place I worked, the place had a policy of any code you push in that does not match the guidelines must be changed. Guidelines was that no em allowed, must use rem. Old part of codebase assigned to me, I found some things in CSS I improved (reuse of code, a small overflow bug) Got comment - you need to change em to be rem. I can't do that because there is em all ove…

My CSS-foo is fairly weak; what problems would em cause such that they disallow it in the code base?

em is relative to the font size of the nearest parent thus you can have situations where your font size of an element node is radically different due to the parent it is placed in. This can of course be powerful if used correctly, but it takes someone really good to use it correctly and perhaps it shouldn't be done in big teams because big teams imply some people not very good in some particular discipline.

on edit: adding this link https://j.eremy.net/confused-about-rem-and-em/

Re: Confessions of a programmer: I hate code review (2010)

#74

Post by carlsborg »

The core message is not "Code reviews are a bad thing" but "Blocked on code review is expensive" and I agree that aspect can be an issue: it can be a badly timed context switch for the reviewer, and we know context switches are expensive in terms of productivity, so in some cases it makes good sense to defer it to a later point in time. And this is valuable time lost for the author.

So whats a better approach?

Re: Confessions of a programmer: I hate code review (2010)

#75

Post by PopeDotNinja »

lmm wrote: post #71

> If I spend a day doing nothing but reading code reviews, I'll end up feeling unsatisfied and unproductive. Because code review feels fundamentally optional -- even though I believe it's beneficial, it's something we've chosen to do, not something that we absolutely have to do in order for the project or business to keep operating -- it's more frustrating to find myself spending a large amount of time on. This is wh…

I'll add that code reviews are vulnerable to garbage in, garbage out. Code reviews the are magically 100% efficient at catching will still be expensive in terms of time if all incoming code is garbage. By reducing upstream garbage, code reviews become less expensive, making more time available for anything else, including coding.

Re: Confessions of a programmer: I hate code review (2010)

#76

Post by miqkt »

I view code reviews as a good opportunity for learning but I've one particular grievance with them. Specifically, when I'm asked to code review a pull request that's alarmingly huge (i.e. touches more than a dozen files, +1000 LOC changes, etc).

I've never come up with a bullet-proof way to deal with this. The situation usually ends up with me providing a rubber stamp of approval and making some caveat commentary about "uncertainties" due to my inability to devote so much time to be rigorous. I feel bad every time I've got to do this.

The problem might be exacerbated at another level too. For features, stories might not be broken down enough.

Re: Confessions of a programmer: I hate code review (2010)

#77

Post by Vinnl »

cimmanom wrote: post #70
Vinnl wrote: post #26

Earlier quoted context omitted.

I think in such a situation, receiving a review is great, even though the downsides mentioned in the article ("what am I going to do while I wait for the review to come in?") are still presents. However, it doesn't do away with how unsatisfying performing a review is. If I spend a day doing code reviews, that is not an enjoyable day to me. This is regardless of how useful I feel it is; it's just not a job I enjoy doi…

While you wait for code review you work on another ticket. What kind of screwy engineering process assumes that code review is instantaneous and doesn’t give you something else to do in parallel?

Nobody expects code review to be instantaneous - that's the problem. Working on another ticket involves context switching, and context switching negatively affects the productivity.

Sure, that downside is probably offset by the benefits of code reviews, but that was exactly what I said: receiving a review is beneficial, it's just a bummer that there's the downside that it's not instantaneous.

Re: Confessions of a programmer: I hate code review (2010)

#78

Post by dotdi »

I used to hate code-reviews but now I'm a convert after: (1) we brought in better people. It's fun to sit with competent people and trash-talk some code, and (2) after I pushed the team to use better tooling, which in our case is GitLab + Merge Requests. You can imagine the dark ages before that.

Re: Confessions of a programmer: I hate code review (2010)

#79

Post by torben-friis »

eridius wrote: post #9

You know what I hate more than code review? Shipping bugs. The author of this piece admits that they have to change the code they write in response to comments. This means the code review is flagging areas of improvement. Surely the resulting code is better than the original code (otherwise I'd expect the author to push back on the comments instead of implementing bad suggestion). Similarly, when acting as the code r…

> The author of this piece admits that they have to change the code they write in response to comments. This means the code review is flagging areas of improvement. Surely the resulting code is better than the original code (otherwise I'd expect the author to push back on the comments instead of implementing bad suggestion).

There's a potential problem with that idea: The assumption that, if several programmers involved offer different solutions, the best solution will prevail.

I think in many cases the accepted solution ends up being the one pushed by the person that holds more power, the one who can communicate better, or the person that 'negotiates' more aggressively.

I'm not saying that to argue against code reviews - It's just a friendly reminder that although as technical people we hope to achieve some objective level of quality, we can't escape office politics, human bias and general subjectivity.

Re: Confessions of a programmer: I hate code review (2010)

#80

Post by watwut »

I hate code review too, mostly power plays it brings. I had reviewer forcing me to produce code I considered bad - but the prolonged discussion before deadline would mean more wasted time so I did not argued. I have seen code review block code for subjective differences in variables naming (e.g. no convention was broken and both names were fine). I have had code reviewer demanding change, after I implemented exactly demanding next change in his own code and after I complained he just shrugged and said "now I think this is better".

I had seen people misuse code review to force standards that they just made up based on blog dude read yesterday - only to change opinion three weeks later.

I had seen senior developer letting junior work alone for two weeks, only to iteratively force the junior to rewrite all of it to seniors liking. Ridiculous waste of time and cruel.

So, with wrong politics and people it is hell.

Post reply on HN