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

41–50 of 50 posts

Re: Ask HN: Do you do code review?

#41

Post by jchung »

matt_the_bass wrote: post #36
kenning wrote: post #31

I work for a tiny 2-man project and we need to save all the time we can, so we don't code review. We actually have an unofficial policy: all code that is directly user-facing (web design for example) is handled by me and all backend stuff is handled by the other guy. My previous company worked the same way though, and it was a real company with funding and customers. It was another tiny startup (essentially 4 person…

With all due respect, your process is likely short sighted. In the long term, you would probably benefit from code reviews. I find code review very effective even in very small teams.

short sighted, but that's sometimes the right way to prioritize. sounds like kenning is in prototyping mode, and could be intentionally prioritizing time to first test, acknowledging that s/he might have to rebuild things later. in the early stages of a startup, speed to insight can easily be more important than long term coding efficiency. so short sighted thinking can make sense.

Re: Ask HN: Do you do code review?

#42

Post by PeterisP »

throwaway84742 wrote: post #38
tempuracat wrote: post #27

How do you motivate people to actually review the code? Not just accept it.

By requiring reviews for merging into master in GitHub and postulating that unless something is in master it does not exist in the product.

What you propose is perfectly compatible with people merely blindly clicking 'Accept" instead of actually reviewing it, which is the problem of the parent poster.

Re: Ask HN: Do you do code review?

#43

Post by avarsh »

We use Crucible [1] for reviews and Jira [2] for task management. Each review links to a Jira task (or handful of related Jira tasks). On each review, we ideally:

- Add 3-4 people as reviewers

- Reviewers ask questions, make suggestions

- Author follows up to these questions (online or offline), and if they make code changes they link to the diff in a comment

- Close reviews only if 2+ people have finished reviewing

- It's okay to close reviews that not everyone has finished reviewing

[1] https://www.atlassian.com/software/crucible

[2] https://www.atlassian.com/software/jira

Re: Ask HN: Do you do code review?

#44

Post by matt_the_bass »

jchung wrote: post #41
matt_the_bass wrote: post #36

Earlier quoted context omitted.

With all due respect, your process is likely short sighted. In the long term, you would probably benefit from code reviews. I find code review very effective even in very small teams.

short sighted, but that's sometimes the right way to prioritize. sounds like kenning is in prototyping mode, and could be intentionally prioritizing time to first test, acknowledging that s/he might have to rebuild things later. in the early stages of a startup, speed to insight can easily be more important than long term coding efficiency. so short sighted thinking can make sense.

I agree with “sometimes”. However from Kennings post it sounded like “always” and that code review is of no value.

Re: Ask HN: Do you do code review?

#45

Post by dmarcos »

Code reviews keep me honest. I know I’m often sloppy and I wan’t to rush things. Code reviews keep me in check. It’s when team work in software development manifests at its best. You need good trust among peers though or it can be destructive. Code reviews are not about looking the smartest or putting others down.

Re: Ask HN: Do you do code review?

#46

Post by karmakaze »

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.

We also believe pair programming is "continuous code-review", but we have found that's not enough on its own. We swap pairs frequently--every week or two to ensure that knowledge isn't siloed in pairs. We also have frequent discussions in varying-sized groups prior to introducing a new service or significant function as well as retros for larger completed units after being in production for a while.

Re: Ask HN: Do you do code review?

#47

Post by kenning »

matt_the_bass wrote: post #44
jchung wrote: post #41

Earlier quoted context omitted.

short sighted, but that's sometimes the right way to prioritize. sounds like kenning is in prototyping mode, and could be intentionally prioritizing time to first test, acknowledging that s/he might have to rebuild things later. in the early stages of a startup, speed to insight can easily be more important than long term coding efficiency. so short sighted thinking can make sense.

I agree with “sometimes”. However from Kennings post it sounded like “always” and that code review is of no value.

> and that code review is of no value.

I don't agree with this at all. Code review is obviously a huge benefit once you have enough money to pay more than one engineer per gigantic slice of the codebase.

I'd say code review is also great if everyone is working on the same thing, such as a library. In that case I'd do code review if there were only two people working on the project. But when one guy is basically just doing react boilerplate and implementing designs, it's not worth the time to check every commit they make.

Re: Ask HN: Do you do code review?

#48

Post by throwaway84742 »

PeterisP wrote: post #42
throwaway84742 wrote: post #38

Earlier quoted context omitted.

By requiring reviews for merging into master in GitHub and postulating that unless something is in master it does not exist in the product.

What you propose is perfectly compatible with people merely blindly clicking 'Accept" instead of actually reviewing it, which is the problem of the parent poster.

Explain the value. Then spot check and separately talk to those who blindly click accept, and if they continue to do so, get new engineers.

Re: Ask HN: Do you do code review?

#49

Post by throwaway84742 »

bostik wrote: post #40
throwaway84742 wrote: post #35

Earlier quoted context omitted.

And moreover, I send my gnarliest PRs to the most vicious reviewer on the team, just because 3 months from now finding out what’s wrong is exponentially more difficult. I want to get it right the first time around, and I want other folks to be able to make changes anywhere in the codebase, even in the gnarly parts.

We have mandatory code reviews for everything, which encourages small, bite-size commits and frequent merges. Majority of the changes goes through with just one other person taking a look. But we also specify (and highly encourage) that anyone trying to land a more invasive or complex change should require multiple review approvals. The most demanding "please pile on, we want eyes on this" request I remember was set…

TBH I favor atomicity over small size. So we get a few gnarly PRs a week. That’s also why we use Reviewable instead of GitHub reviews

Re: Ask HN: Do you do code review?

#50

Post by matt_the_bass »

kenning wrote: post #47
matt_the_bass wrote: post #44

Earlier quoted context omitted.

I agree with “sometimes”. However from Kennings post it sounded like “always” and that code review is of no value.

> and that code review is of no value. I don't agree with this at all. Code review is obviously a huge benefit once you have enough money to pay more than one engineer per gigantic slice of the codebase. I'd say code review is also great if everyone is working on the same thing, such as a library. In that case I'd do code review if there were only two people working on the project. But when one guy is basically just…

Thanks for your clarification.
Post reply on HN