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

131–140 of 165 posts

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

#131

Post by cimmanom »

Vinnl wrote: post #124
cimmanom wrote: post #110

Earlier quoted context omitted.

You're going to be context switching at the point you make the pull request anyway. And if you structure your day well, you can do the rest of the context switching at a point when it's not a burden anyway - such as making changes in response to code review first thing in the morning before returning to your other tickets.

Well, I guess we'll have to agree to disagree there. It's far easier to continue working on the same thing, even if I have to make a pull request (or simply have to write a commit) in between, as long as that's still related to the same change. If I have to switch between entirely different problems, that has more impact on my productivity than having to switch tasks while still working on the same problem. Again, in…

This still sounds to me like it might be a process issue around code review rather than code review actually being the problem. What else would you be doing on the same problem post-PR if you didn’t have to wait for code review?

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

#132

Post by UncleMeat »

lostcoder wrote: post #82
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…

I'm fortunate enough at my company to not have to submit to the code review bureaucracy - and I can justify it by having metrics which indicate that my code ships with fewer bugs than the code of other teams which do go through code reviews. I think everyone should have some level of freedom to work in a manner which works best for them, rather than forcing everyone into the same process. Success should be measured b…

You are on a team though. If you are better than everybody else then your team is missing out on your code reviews.

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

#133

Post by rabidrat »

barbs wrote: post #25
zaksoup wrote: post #6

I've found that favoring full time pair programming and not reviewing work that was soloed upon unless the author explicitly asked for it because of self-identified uncertainty or caution has resulted in a better workflow with little to no actual hit to quality. Additionally, proper test-driving and testing practices are incredibly effective ways to enforce high quality code. Again, this comes down to culture and pra…

Pair-programming 100% of the time sounds like a nightmare to me. I need uninterrupted focus - something I can't do with someone else there constantly, even if we're working on the same thing

Have you actually tried it? It's different than you think. (At least it has been for me).

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

#135

Post by distances »

tekkk wrote: post #112
LandR wrote: post #95

Earlier quoted context omitted.

I don't get this. A code review is a purely technical process. If he says your code is bad / wrong etc then that is a purely technical discussion that has to be had if you feel they are wrong. Why are you looking for empathy in code reviews? Why are you getting feelings involved at all?

This might had been a cultural thing between me and the other party. I don't mind being very to the point and strict about programming but when it comes off as condescending and "I know better than you" it's just something I can't stand. Or maybe a better example, they make pesky remarks on your programming style while ignoring all your writing about theirs. I think because the other person here was a bit older than…

As you already said, this sounds like a people issue. Snark, sarcasm or belittling has no place whatsoever in a code review (or at a work setting in general, obviously). Sounds like you just got a bad colleague.

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

#136

Post by distances »

miqkt wrote: post #76

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 abo…

At one project there was a rule for large PRs: if the change exceeds a thousand lines, you have to bring review cookies. Keeps you mindful of the PR size and the rest of the team won't grumble that much.

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

#137

Post by kqr »

rasteau wrote: post #60
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…

Yikes! Demanding a bug fix also remediate unrelated, extant style violations is a good way to ensure nothing improves.

There is something to be said for continuous improvement (my company has a policy of leaving the campground neater than when we got there, which is one of the reasons I enjoy working there so much), but there is a time and place for everything...

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

#138

Post by regularfry »

petetnt wrote: post #29
regularfry wrote: post #17

The problem with code review is that it works. That's why we can't shake the industry's fascination with it. It works to catch bugs, but it's horrendously inefficient in doing so. I ran some numbers on our code review process on a project last year. Of patches returned for modification, 5% contained a bug. 95% were for entirely stylistic changes. I find it hard to square that with being a good use of time. As an indu…

First step to get right would be having automatic code formatters in place properly to cut out stylistic nits. With tools like Prettier, gofmt, yapf, Black and similar you can focus on the code itself, not how it looks. Of course it can take a while to find some common ground to some stylistic options (which is why I love opinionated formatters that don't give the choice) but it pays off quickly regardless and you ga…

That takes care of the lowest-level stylistic issues, but doesn't capture things like "why are we trapping errors here rather than in a central error handler there". The sort of question where the app will function either way, but reasonable people can have aesthetic disagreements, and a formatter is totally useless.

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

#139

Post by erwan »

Vinnl wrote: post #101
bradleyjg wrote: post #99

Earlier quoted context omitted.

There are definite upsides to having a detailed written record of a code review, but in terms of enjoyment if I have some code to review I read though it, take some notes, grab a meeting room and have a discussion with the person that wrote the code. I find it far less tedious than having to write up my critiques in detail. Then the written review in the tool can just be terse reminders to the things we discussed. I'…

Hmm, that does sound like a good idea that would make it a lot more enjoyable, with the added bonus of being able to use tone of voice and facial expressions to prevent remarks from being taken the wrong way. I'm not currently in a position where I share an office with people whose code I review, but when I am again, I definitely want to try this. Thanks for sharing!

I want to second what OP said. A lot of the pain points people have from code review seem to come from limiting themselves to the review tool to understand the aim, scope, and rational behind some decisions. You should definitely use other channels (1-1 discussion, grabbing coffee, a meeting room, a call, whatever works).

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

#140

Post by j88439h84 »

kqr wrote: post #93
DubiousPusher wrote: post #21

Some simple tips I use to make this better. 1) Anything cosmetic gets a "nit" added to the comment. 2) When a review looks like mostly assets or boiler plate, I look through quickly for anything glaring and approve while noting my assumptions that the author has run the build and verified the additions. 3) If I'm in a hurry and the author is available I grab them and do an over the shoulder review. You might be surpr…

I think these points are very salient. I'd suggest adding another one: create a checklist for code review that is relevant to the issues your team faces. The checklist makes it much easier for you as a reviewer to perform evenly.

Great idea, thanks!
Post reply on HN