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

Ask HN: Do you do code review?

news.ycombinator.com

Post reply on HN
Print view View original

11–20 of 50 posts

Re: Ask HN: Do you do code review?

#11

Post by ruffrey »

At a medium sized company. Everything gets code reviewed by one or two teams. Only one or two approvals are required. Stash/bitbucket is used and we leverage the Default Reviewers feature. The process works very well, is integrated with our ticketing system (Jira) and build system (Bamboo). We are nice in the code reviews but thorough and a little pedantic about style.

The only reason I can picture not doing code review is if there is only one developer. It is an excellent way for everyone to learn, reduce bugs, reduce spaghetti code, force you to write tests, etc.

Re: Ask HN: Do you do code review?

#12

Post by hennsen »

Yes i do whenever possible - that is, if the team i work with as a freelancer already does or is willing to follow my advice to do it. That is - disturbingly - not always the case.

I only worked with gitlab abd github pull/merge request code review tooling and found it ok. There’s Gerrit, but i had no chance to work with it yet. Obviously always depends on how they need to be done. When in doubt, just sit next to each other and look at the part of code and changes for the current task your working on that is about to be merged. Not always a special tool necessary and if you don’t know which one to use better just start than let the tool choice get in the way. That being said I’m not against tools!

Suggestions? As with any process or meeting, define what the scope is! Is it about coding style? Which are your guidelines for that one then? Is it about getting a second look if the story is implemented in a good, efficient way? Is it to verify definition of done rules are met? About having (good) tests? It’s probably not (counterexample i just had) to discuss the actual requirements defined by the story the code should fulfill... that should have happened at another time, probably with other persons, like the Product Owner. How often? I think it’s best to do it with each merge request for the code it contains, and maybe in defined tinespans for the whole project.

Re: Ask HN: Do you do code review?

#13

Post by rakoo »

mengledowl wrote: post #3

100% would recommend doing code reviews. The most effective way I've found to do code reviews is to create a pull request for the code you would like to have reviewed. Send it out to someone or a few people that you would like to take a look, get them to approve or reject it, and then take a look at their comments and see what you can do to address them. Some guidelines for code reviews: 1. Build each other up. The p…

Seconded. Code reviews obviously helps for catching the small bugs/inefficiencies that the submitter hasn't thought of, but a surprising side-effect of having mandatory code reviews is that it actually helps everyone learn about the system as a whole, together. There's no better way to learn than to see someone (sometimes even yourself!) try something, hear that it's not correct, and see the fix. The failing and recovering steps are sometimes more helpful than just seeing "this is how we do it"-comments.

Regarding the big diffs: sometimes it's inevitable because you're doing a big formatting of the code, or you're just doing a no-op refactoring introducing new primitives. If you do so, separate commits with one being the refactoring with no functional changes, and one being the actual changes. Makes review much easier.

Re: Ask HN: Do you do code review?

#14

Post by matfil »

I suspect this may not be too well received, but I've thus far managed to avoid regular formal reviews, and I'd advise others to think twice before imposing them. They seem like a big step towards treating programmers like cogs in a machine rather than competent individuals, and for me that's a direction I don't want to be headed.

Re: Ask HN: Do you do code review?

#15

Post by mavsman »

Spartan-S63 wrote: post #6

We don't do code reviews because we pair program 100% of the time. Our belief is that pairing is like embedding a code review in the process.

I can appreciate this philosophy but I don't think it's right for everyone. On the one hand, it's nice to have a safety net for times when pairing turns into one person programming while the other is spacing out. On the other hand, if you require a review on top of pairing then you're now requiring 3 people to get this chunk of code out. So there's definitely a balance and you have to decide what's right depending on your team and situation.

Re: Ask HN: Do you do code review?

#16

Post by http-teapot »

mengledowl wrote: post #3

100% would recommend doing code reviews. The most effective way I've found to do code reviews is to create a pull request for the code you would like to have reviewed. Send it out to someone or a few people that you would like to take a look, get them to approve or reject it, and then take a look at their comments and see what you can do to address them. Some guidelines for code reviews: 1. Build each other up. The p…

This is a great list!

I'd like to suggest one if you don't mind

7. Review your own code before handing it over. Time is wasted when the submitter left out obvious issues such as typos, missing comments, unnecessary complexity, debug code, etc.

Consider the reviewer's time precious because context switching is expensive and she/he might not get back to it as quick as you'd like to.

Implementing clear guidelines and linting would help.

Re: Ask HN: Do you do code review?

#17

Post by ccharles »

matfil wrote: post #14

I suspect this may not be too well received, but I've thus far managed to avoid regular formal reviews, and I'd advise others to think twice before imposing them. They seem like a big step towards treating programmers like cogs in a machine rather than competent individuals, and for me that's a direction I don't want to be headed.

Have you tried doing them?

Other users have already described healthy code review processes that improve code quality while helping all participants to grow individually and as a team.

How does review treat programmers like cogs in a machine?

Re: Ask HN: Do you do code review?

#19

Post by seattle_spring »

Spartan-S63 wrote: post #6

We don't do code reviews because we pair program 100% of the time. Our belief is that pairing is like embedding a code review in the process.

I would literally rather be unemployed than have to interact with someone for the full 8 hours every day.

Re: Ask HN: Do you do code review?

#20

Post by geofft »

matfil wrote: post #14

I suspect this may not be too well received, but I've thus far managed to avoid regular formal reviews, and I'd advise others to think twice before imposing them. They seem like a big step towards treating programmers like cogs in a machine rather than competent individuals, and for me that's a direction I don't want to be headed.

I find that good programmers will voluntarily request reviews whether or not they are required by process, and bad programmers will attempt to minimize them even when required by process. Having a policy is a good way to weed out the bad people, and won't affect the good people.
Post reply on HN