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

Code reviews aren’t just for catching bugs

blog.fullstory.com

Post reply on HN
Print view View original

21–30 of 150 posts

Re: Code reviews aren’t just for catching bugs

#21

Post by mikestew »

eduren wrote: post #12

Working at a Gov't IT contractor, I really wish we had the organizational skills/incentive to do code reviews. Very rarely does the code I write get glanced at, much less examined. My instinct, of course, is to solicit code reviews from my peers, but the organizational structure and support tooling are all woefully inadequate. Plenty of projects, even greenfield ones, aren't checked into source control. And of course…

Plenty of projects, even greenfield ones, aren't checked into source control.

I really and truly did not know this still happens. Hell, even on throwaway/PoC stuff for which I am the sole developer, and code that stands a good chance of never seeing the light of day, I start with git init. 'cuz the probability that I'm going to wish later that it was in source control outweighs the very minor cost of putting it in there. For an organization that produces software that others will later use, I'm at a loss to explain it other than inertia.

Re: Code reviews aren’t just for catching bugs

#22

Post by collyw »

eduren wrote: post #12

Working at a Gov't IT contractor, I really wish we had the organizational skills/incentive to do code reviews. Very rarely does the code I write get glanced at, much less examined. My instinct, of course, is to solicit code reviews from my peers, but the organizational structure and support tooling are all woefully inadequate. Plenty of projects, even greenfield ones, aren't checked into source control. And of course…

Hardly any one gives a shit about code quality, except the developers that have to maintain it afterwards.

Saying that, poor code quality has been a real incentive for me to improve my own code. When I eventually work out what something is doing and see how much simpler it could be i try to redo it in a way that will be a lot more understandable to the next person. If I visit my own code a month or two later and don't immediately understand it, its time to refactor it. At the end of the day I prefer writing code to debugging it, so the less time I have to spend trying to understand code, the less time I spend debugging.

Re: Code reviews aren’t just for catching bugs

#23

Post by dmarg »

eric_h wrote: post #6
buro9 wrote: post #2

I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review. I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct p…

I work on a very small team (4 engineers), so we have all commits for all branches posted to our engineering chatroom. Everyone knows what everyone else is working on (and we all work in the same room), so when someone is on a task that we know my have some snags/difficult-to-design solutions, we'll all periodically take a break and look at decisions other people are making on their particular feature, providing feed…

I work on a team with the same size as yours. Although we do not all sit in the same room, we still do code reviews. Since we are small we are not the most critical on style.

I am a big believer in code reviews especially starting early with a small team. This would set the culture from the beginning because it is harder to bring that in later.

Re: Code reviews aren’t just for catching bugs

#25

Post by draw_down »

fishnchips wrote: post #5
buro9 wrote: post #2

I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review. I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct p…

This is the role of design documents which get heavily reviewed by the team before even the first line of code is written.

We write those on my team. They usually end up only faintly resembling what actually gets shipped. No matter how much agreement there is beforehand, as soon as people see working software they change their minds. The best thing is to plan for and expect change, not try to mitigate its manifesting.

Re: Code reviews aren’t just for catching bugs

#26

Post by gracenotes »

As several other commenters have pointed out, code reviews are not a silver bullet. It is widely known that there is no silver bullet.

That said, if your goal is to make a quality product, you shouldn't have to choose between code reviews and continuous integration. You shouldn't have to choose between code reviews and code coverage, or manual QA processes. These are all widely regarded as best practices and if implemented "correctly" and appropriately to the team their combination forms a virtuous cycle for code health and team culture.

Re: Code reviews aren’t just for catching bugs

#28

Post by chipgap98 »

msoad wrote: post #20

Code reviews can easily become a tool for people with huge egos to prove their smartness. I get code review comments for grammar of my comments or very small code style preferences that Google's anal style guide can't enforce (yet). I like code reviews, don't get me wrong. But there should be a way to respond with "you just shut up, you're only trying to make yourself look smart". It's all because higher up people mo…

That sounds like a culture problem. You don't really want people who are out to prove how smart they are, versus people that are trying to help the other engineers they work with get better.

Re: Code reviews aren’t just for catching bugs

#29

Post by wellpast »

eduren wrote: post #18
wellpast wrote: post #11

What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. Most of the "problems" that code reviews claim to address can be solved by much more direct and optimal measures. Code reviews are damn expensive. This post concedes that code reviews are better for the more fluffy ends -- te…

>This post concedes that code reviews are better for the more fluffy ends -- teamwork, openness, social recognition, but given their high costs, I'd rather achieve even these soft goals in other ways than to impede my team's delivery potential. What techniques have you found effective for improving the soft goals in the context of software (genuinely curious)? Are we talking more conventional management/business conc…

Not sure how to formalize but the team's I've worked on have generally negotiated mutual respect, openness, and teamwork by collaborating on the things of greater import -- the architecture, domain conceptualization, etc. And peer code reviews in some cases tend to work against these goals--because you tend to be down in the weeds of LOC, bike-shedding, arguing over the equivalents of tabbing and spacing or whether a line could be more functionally expressed, e.g....

Re: Code reviews aren’t just for catching bugs

#30

Post by [deleted] »

chipgap98 wrote: post #28
msoad wrote: post #20

Code reviews can easily become a tool for people with huge egos to prove their smartness. I get code review comments for grammar of my comments or very small code style preferences that Google's anal style guide can't enforce (yet). I like code reviews, don't get me wrong. But there should be a way to respond with "you just shut up, you're only trying to make yourself look smart". It's all because higher up people mo…

That sounds like a culture problem. You don't really want people who are out to prove how smart they are, versus people that are trying to help the other engineers they work with get better.

[deleted]
Post reply on HN