5 ms·

I'm glad this works better for them, or at least they feel that it does. I'd also bet that they're either a small minority of teams that jell so well that they

I'm glad this works better for them, or at least they feel that it does. I'd also bet that they're either a small minority of teams that jell so well that they manage to scale a codebase without it turning into a disgustingly inconsistent mess; or that indeed their codebase qualifies as such or is still too small for the effects to be very visible. Code reviews are not a tool intended mainly to catch bugs. Types, automated tests and manual tests are going to do the bulk of your bug-catching after all. Code reviews' more relevant purpose is to act as QA for the code itself far more than it is to verify if the code achieves what it sets out to do. This is in service of both maintainability (which impacts your ability to make future changes and staff retention) as well as performance (by introducing the chance for another dev to detect suboptimal approaches). Once your teams grow, code piles up, and static analysis can't cover all your bases, either your team corroborates the code that you write matches/defines group standards; or each developer's careful design accumulates as "organic growth" carefully duct taped together to barely work. And just to state the obvious, code reviews are merely a tool to achieve this. Pair programming is another.

commandlinefan 5y ago
> Code reviews are not a tool intended mainly to catch bugs Well, not to detract from your point (I agree), but what little research has been done in this area suggests that code reviews are actually one of the best ways to catch bugs: https://kevin.burke.dev/kevin/the-best-ways-to-find-bugs-in-your-code/ https://kevin.burke.dev/kevin/the-best-ways-to-find-bugs-in-...
- kubanczyk 5y ago
  The source actually says that informal code reviews are very poor way to catch bugs, while formal code reviews are one of the best. I don't have "Code Complete" at hand to follow up on that, but I strongly suspect that the formal code review was defined in this research as reviewing the entire codebase line-by-line on meetings. As opposed to reviewing just a single change to the system. Can you/anyone shed some light on this?
  - deleted 5y ago
    [deleted]
  - Fishkins 5y ago
    My reading is that a formal code review is still focused on a single change, but with a more explicit/required process for review. This sentence in the article backs that up: > Before any code at Google gets checked in, one owner of the code base must review and approve the change (formal code review and design inspection) That's clearly referring to a specific change, not a review of the whole codebase. I used to work for a large company. They have homegrown review software with extensive checklists of criteria the code must meet before it can pass review. The software won't let the code progress unless two reviewers go through and explicitly check off every box to certify it meets their standards. I'd call that formal code review. I work on a smaller team now. We do consistently perform code reviews, but there's nothing actually preventing someone from merging without a review. We also have a list of things to look for when performing a code review, but nothing forces reviewers to look at the document regularly. That process is closer to an informal code review. I still find bugs regularly when reviewing code. That's both true of other people's PRs as well as my own. I imagine there's a lot more variability with an informal process than a formal one, but IMO informal review is still useful. Code review of course also provides benefits other than bug detection. I always review my own PR before passing it to someone else. Another commenter recommended that practice, and I heartily endorse it.
whoomp12342 5y ago
IMO code reviews are a tool for mentorship and keeping architecture in line
- ipaddr 5y ago
  They can also be used for bullying, micromanaging and other social constructs
  - xboxnolifes 5y ago
    something, something, quote about all tools being able to be misused.

commandlinefan 5y ago

> Code reviews are not a tool intended mainly to catch bugs Well, not to detract from your point (I agree), but what little research has been done in this area suggests that code reviews are actually one of the best ways to catch bugs: https://kevin.burke.dev/kevin/the-best-ways-to-find-bugs-in-your-code/ https://kevin.burke.dev/kevin/the-best-ways-to-find-bugs-in-...

kubanczyk 5y ago
The source actually says that informal code reviews are very poor way to catch bugs, while formal code reviews are one of the best. I don't have "Code Complete" at hand to follow up on that, but I strongly suspect that the formal code review was defined in this research as reviewing the entire codebase line-by-line on meetings. As opposed to reviewing just a single change to the system. Can you/anyone shed some light on this?
- deleted 5y ago
  [deleted]
- Fishkins 5y ago
  My reading is that a formal code review is still focused on a single change, but with a more explicit/required process for review. This sentence in the article backs that up: > Before any code at Google gets checked in, one owner of the code base must review and approve the change (formal code review and design inspection) That's clearly referring to a specific change, not a review of the whole codebase. I used to work for a large company. They have homegrown review software with extensive checklists of criteria the code must meet before it can pass review. The software won't let the code progress unless two reviewers go through and explicitly check off every box to certify it meets their standards. I'd call that formal code review. I work on a smaller team now. We do consistently perform code reviews, but there's nothing actually preventing someone from merging without a review. We also have a list of things to look for when performing a code review, but nothing forces reviewers to look at the document regularly. That process is closer to an informal code review. I still find bugs regularly when reviewing code. That's both true of other people's PRs as well as my own. I imagine there's a lot more variability with an informal process than a formal one, but IMO informal review is still useful. Code review of course also provides benefits other than bug detection. I always review my own PR before passing it to someone else. Another commenter recommended that practice, and I heartily endorse it.

whoomp12342 5y ago

IMO code reviews are a tool for mentorship and keeping architecture in line

ipaddr 5y ago
They can also be used for bullying, micromanaging and other social constructs
- xboxnolifes 5y ago
  something, something, quote about all tools being able to be misused.