5 ms·

Out of curiosity, what's the context in which you think developers should stop doing code reviews after 3-6 months? Every place I've worked has required code re

Out of curiosity, what's the context in which you think developers should stop doing code reviews after 3-6 months? Every place I've worked has required code reviews for all changes (except in emergencies or when the author thinks the change is trivial). For example, how big of a company are you talking about, and how critical is correctness and maintainability? My impression is that skipping code reviews mostly makes sense for early startups, internal tools, prototypes, and other situations where velocity is more important than quality, but that most companies should have pretty much all code go through review.

valuearb 9y ago
I can’t imagine any reason to skip code reviews in any professional development. Someone is paying the team to produce professional code, finding out well after the fact that one section/contributed failed that test is unacceptable. Code reviews shouldn’t be lots of overhead. They shouldn’t devolve into style critiques. They should be the best tool you have for teaching and communicating best practices. Reviewing a more senior devs code can teach a junior a lot, and occasionally catch significant errors.
- sharkdesk 9y ago
  Actually, someone is paying the team to solve business problems and create revenue.
  - Clubber 9y ago
    Preferably in the most efficient way possible (which is of course subjective). I believe that after 3-6 months in a particular subsystem, the ROI of code reviews increasingly decreases over time.
    - Anticapitalist 9y ago
      But shouldn't the time required to review also drop significantly when the reviewer is familiar with the module?
  - valuearb 9y ago
    The people who pay my team mostly want bugs fixed. Bugs going in testing are far more expensive to fix than those found in code review.
    - Clubber 9y ago
      >Bugs going in testing are far more expensive to fix than those found in code review. How so? Particularly if you don't do your code review right after development. That holds true for production vs. QA, but I'm not sure it does for QA vs. code review, unless your QA deployment and ticket bounce back process is overly cumbersome and you don't have time planned for it.
      - valuearb 9y ago
        We always do code review before QA. But our QA process is also multilayered, first layer is QA engineer, next layer is first level business acceptance testing, then another higher level business acceptance testing. Gets pretty expensive once the SR marketing person rejects. No i didn’t design this process.
- watwut 9y ago
  "They should be the best tool you have for teaching and communicating best practices. " For christ sake, no. If you want communicate new best practice, do so. And if team agrees, we will follow. Don't ambush me with new expectations during code review and don't keep my task hostage in order to push for your new best practice. Same with teaching. Teach proactively instead of pretending juniors can solve it and then rewrite all their decisions in code rewiew. If you have expectations, tell them in advance. It is just unfair to use code review for that.
  - Clubber 9y ago
    That reminds me of a SQL code reviewer at my last company. He'd have a standard that we would follow and finally get stuff passed most of the time. After a few weeks, he would read a new article and completely change his passing criteria without telling anyone, so all of our previously passing submissions would then fail. This happened several times over the course of a year. It was very frustrating and rarely helped the overall quality of the SQL.
  - alangpierce 9y ago
    > If you want communicate new best practice, do so. I think the quote your replying to means existing best practices. IMO, things like "we tend to follow this naming convention" and "here's a useful language feature you may not know about" seem completely fine to say at code review time, and it's not like you can teach someone absolutely everything before they get started. Having a reviewer hold a review hostage is a big cultural red flag, IMO. If there's a real significant disagreement, you shouldn't try to solve it via code review comments; you should move to in-person communication if possible and maybe pull other people in to take a look. I also think that in most situations, reviewers should bias toward suggestions and ideas rather than demands ("strong opinions, weakly held"), and authors should feel comfortable pushing back or proposing to merge the commit as-is and leave some change for a follow-up review. Saying code reviews are "the best tool" for teaching and communicating best practices is maybe a stretch; I find things like pair programming to be much better for stuff like that. But I still think code reviews can be a great mechanism for teaching code-level details like code style.
  - valuearb 9y ago
Clubber 9y ago
I think they lose their value in any organization. It's not a hard rule, because there are a lot of variables that could increase or decrease their value. I'll give you an example of each: 1. Decreased value: If a person is the primary (only) developer on a subsystem, any outside code reviewer will be less informed of the subsystem he is reviewing. I don't see any value in that other than get the other reviewer up to date on a subsystem he doesn't actively develop / maintain. I guess you could argue redundancies, but if you have a big system, having every developer understand every part of the system is unrealistic. 2. Increased value: if a developer is working on a new subsystem they aren't familiar with, then the 3-6 month window restarts. The 3-6 month window isn't the time of employment, it's really the time of understanding a particular subsystem. My point is that doing code reviews in and of itself has decreasing value after 3-6 months. The 3-6 months is the time I'm estimating for a competent developer to familiarize himself with an existing subsystem codebase. Also, any developer that still has issues after the 3-6 window of the same subsystem is a huge red flag. Code reviews will then just be a crutch for him and the entire organization when the real solution would be to get a better developer. Also, I'm not saying any communication about code loses value. If I want a guy to look at my code and help me work though something (usually by just listening to me explain it), that has value. I'm generally speaking of the formalized code review that must happen after any coding change. That loses value after a developer is acclimated to the subsystem he has been working on, especially if the reviewer doesn't know as much about the subsystem he is reviewing (and has no need to know). I'm a very anti-hard-process guy. A process must have strong value, otherwise it's just another speed bump.

valuearb 9y ago

I can’t imagine any reason to skip code reviews in any professional development. Someone is paying the team to produce professional code, finding out well after the fact that one section/contributed failed that test is unacceptable. Code reviews shouldn’t be lots of overhead. They shouldn’t devolve into style critiques. They should be the best tool you have for teaching and communicating best practices. Reviewing a more senior devs code can teach a junior a lot, and occasionally catch significant errors.

sharkdesk 9y ago
Actually, someone is paying the team to solve business problems and create revenue.
- Clubber 9y ago
  Preferably in the most efficient way possible (which is of course subjective). I believe that after 3-6 months in a particular subsystem, the ROI of code reviews increasingly decreases over time.
  - Anticapitalist 9y ago
    But shouldn't the time required to review also drop significantly when the reviewer is familiar with the module?
- valuearb 9y ago
  The people who pay my team mostly want bugs fixed. Bugs going in testing are far more expensive to fix than those found in code review.
  - Clubber 9y ago
    >Bugs going in testing are far more expensive to fix than those found in code review. How so? Particularly if you don't do your code review right after development. That holds true for production vs. QA, but I'm not sure it does for QA vs. code review, unless your QA deployment and ticket bounce back process is overly cumbersome and you don't have time planned for it.
    - valuearb 9y ago
      We always do code review before QA. But our QA process is also multilayered, first layer is QA engineer, next layer is first level business acceptance testing, then another higher level business acceptance testing. Gets pretty expensive once the SR marketing person rejects. No i didn’t design this process.
watwut 9y ago
"They should be the best tool you have for teaching and communicating best practices. " For christ sake, no. If you want communicate new best practice, do so. And if team agrees, we will follow. Don't ambush me with new expectations during code review and don't keep my task hostage in order to push for your new best practice. Same with teaching. Teach proactively instead of pretending juniors can solve it and then rewrite all their decisions in code rewiew. If you have expectations, tell them in advance. It is just unfair to use code review for that.
- Clubber 9y ago
  That reminds me of a SQL code reviewer at my last company. He'd have a standard that we would follow and finally get stuff passed most of the time. After a few weeks, he would read a new article and completely change his passing criteria without telling anyone, so all of our previously passing submissions would then fail. This happened several times over the course of a year. It was very frustrating and rarely helped the overall quality of the SQL.
- alangpierce 9y ago
  > If you want communicate new best practice, do so. I think the quote your replying to means existing best practices. IMO, things like "we tend to follow this naming convention" and "here's a useful language feature you may not know about" seem completely fine to say at code review time, and it's not like you can teach someone absolutely everything before they get started. Having a reviewer hold a review hostage is a big cultural red flag, IMO. If there's a real significant disagreement, you shouldn't try to solve it via code review comments; you should move to in-person communication if possible and maybe pull other people in to take a look. I also think that in most situations, reviewers should bias toward suggestions and ideas rather than demands ("strong opinions, weakly held"), and authors should feel comfortable pushing back or proposing to merge the commit as-is and leave some change for a follow-up review. Saying code reviews are "the best tool" for teaching and communicating best practices is maybe a stretch; I find things like pair programming to be much better for stuff like that. But I still think code reviews can be a great mechanism for teaching code-level details like code style.
- valuearb 9y ago

Clubber 9y ago

I think they lose their value in any organization. It's not a hard rule, because there are a lot of variables that could increase or decrease their value. I'll give you an example of each: 1. Decreased value: If a person is the primary (only) developer on a subsystem, any outside code reviewer will be less informed of the subsystem he is reviewing. I don't see any value in that other than get the other reviewer up to date on a subsystem he doesn't actively develop / maintain. I guess you could argue redundancies, but if you have a big system, having every developer understand every part of the system is unrealistic. 2. Increased value: if a developer is working on a new subsystem they aren't familiar with, then the 3-6 month window restarts. The 3-6 month window isn't the time of employment, it's really the time of understanding a particular subsystem. My point is that doing code reviews in and of itself has decreasing value after 3-6 months. The 3-6 months is the time I'm estimating for a competent developer to familiarize himself with an existing subsystem codebase. Also, any developer that still has issues after the 3-6 window of the same subsystem is a huge red flag. Code reviews will then just be a crutch for him and the entire organization when the real solution would be to get a better developer. Also, I'm not saying any communication about code loses value. If I want a guy to look at my code and help me work though something (usually by just listening to me explain it), that has value. I'm generally speaking of the formalized code review that must happen after any coding change. That loses value after a developer is acclimated to the subsystem he has been working on, especially if the reviewer doesn't know as much about the subsystem he is reviewing (and has no need to know). I'm a very anti-hard-process guy. A process must have strong value, otherwise it's just another speed bump.