3 ms·

Instead of structuring your team around the architecture of the software, you architect the software around the structure of your team. It is a corrolary of Con

Instead of structuring your team around the architecture of the software, you architect the software around the structure of your team. It is a corrolary of Conways law. Promote selfishness (call it ownership) and get just get rid of code review. As you may imagine, this approach has fat tail risk, but that is not necessarily a problem in a high failure rate environment.

shakezula 4y ago
> just get rid of code review You’ve lost me there and will never get me back. I can’t abide any workflow that doesn’t put code review as top priority, regardless of remote or in person, sync or async.
- lamontcg 4y ago
  A lot of code review is needless nitpicking that doesn't help to produce bug free software and mostly provides friction. Particularly with senior devs reviewing other senior devs work. I don't think I'd say that code review needs to be abolished, but it often needs to be put on a bit of a diet. And I've seen PRs where code beautification feedback has gotten so out of hand and the PR is now 10% code fixes and 90% unrelated cleanup in the class/module/whatever that I can no longer determine if its safe to merge or not and that no regressions were introduced by the code review process itself.
  - orwin 4y ago
    > I don't think I'd say that code review needs to be abolished, but it often needs to be put on a bit of a diet. To clarify: You mean whole team meeting for code-review needs a diet? Because i agree. I actually think code review is always positive in pair programming situation, and often a waste of time in a team meeting (even with smallish teams). Maybe include the new dev during the code review just to witness (and ask questions later for his enboarding).
    - lamontcg 4y ago
      Uh no, I mean the doctrine of code review in general needs to be put on a diet. I'm definitely opposed to everyone sitting around synchronously nitpicking everyone else's code in enforced meetings, but I wasn't considering that. Onboarding new devs is where code review and pair programming is obviously useful. And honestly I've seen it go both ways, where onboarding a new dev was an opportunity for the old veterans to learn some new tricks. If code review for new devs is just beating them into submission with your own code standards for the sake of your own code standards, then it becomes more of an ego-flexing exercise. But day-to-day it can get very repetitive and if I'm looking at a PR by a dev who has been around for a long time, doing work which is repetitive work that we've already beaten to death how to do it all correctly, I'm going to give it a very thin skim--that would probably absolutely horrify people who deify code review--before just approving it. And most of the regressions that I've seen shipped went through code review and everyone missed the edge condition, and done 100 more times everyone would have missed it again, and nobody would have seen the deficiency in the tests. Donald Rumsfeld's unknown-unknowns. This obviously doesn't apply at all to externally-contributed code to open source software, because you can't really trust external contributors to think through all the edge conditions, you have to assume they're operating in "just-fix-my-bug" mentality and the fix may not be correct or may not be (adequately) tested. That's where you need to be very defensive and need to have your "steward of the codebase" hat on. But for teammates that you've worked with for years, everyone should have their own hat pretty well-developed and its good for velocity and reduction of frustration to trust them a lot more.
      - shakezula 4y ago
        > looking at a PR by a dev who has been around for a long time This is precisely the case where I think you need code review the most, because in my experience, time makes people complacent. Code review is peer review, the key agent of the scientific method. I just can’t abide a process that doesn’t include peer review. > Donald Rumsfeld's unknown-unknowns. are an excellent reason to have others question your code. Code review is a culture issue. If you have people who are so unfamiliar with code as to not be able to review it, then that’s a clear knowledge silo that seems perfectly addressable by _code review_. It’s not about the code, it’s about the knowledge transfer and peer enforcement.

shakezula 4y ago

> just get rid of code review You’ve lost me there and will never get me back. I can’t abide any workflow that doesn’t put code review as top priority, regardless of remote or in person, sync or async.

lamontcg 4y ago
A lot of code review is needless nitpicking that doesn't help to produce bug free software and mostly provides friction. Particularly with senior devs reviewing other senior devs work. I don't think I'd say that code review needs to be abolished, but it often needs to be put on a bit of a diet. And I've seen PRs where code beautification feedback has gotten so out of hand and the PR is now 10% code fixes and 90% unrelated cleanup in the class/module/whatever that I can no longer determine if its safe to merge or not and that no regressions were introduced by the code review process itself.
- orwin 4y ago
  > I don't think I'd say that code review needs to be abolished, but it often needs to be put on a bit of a diet. To clarify: You mean whole team meeting for code-review needs a diet? Because i agree. I actually think code review is always positive in pair programming situation, and often a waste of time in a team meeting (even with smallish teams). Maybe include the new dev during the code review just to witness (and ask questions later for his enboarding).
  - lamontcg 4y ago
    Uh no, I mean the doctrine of code review in general needs to be put on a diet. I'm definitely opposed to everyone sitting around synchronously nitpicking everyone else's code in enforced meetings, but I wasn't considering that. Onboarding new devs is where code review and pair programming is obviously useful. And honestly I've seen it go both ways, where onboarding a new dev was an opportunity for the old veterans to learn some new tricks. If code review for new devs is just beating them into submission with your own code standards for the sake of your own code standards, then it becomes more of an ego-flexing exercise. But day-to-day it can get very repetitive and if I'm looking at a PR by a dev who has been around for a long time, doing work which is repetitive work that we've already beaten to death how to do it all correctly, I'm going to give it a very thin skim--that would probably absolutely horrify people who deify code review--before just approving it. And most of the regressions that I've seen shipped went through code review and everyone missed the edge condition, and done 100 more times everyone would have missed it again, and nobody would have seen the deficiency in the tests. Donald Rumsfeld's unknown-unknowns. This obviously doesn't apply at all to externally-contributed code to open source software, because you can't really trust external contributors to think through all the edge conditions, you have to assume they're operating in "just-fix-my-bug" mentality and the fix may not be correct or may not be (adequately) tested. That's where you need to be very defensive and need to have your "steward of the codebase" hat on. But for teammates that you've worked with for years, everyone should have their own hat pretty well-developed and its good for velocity and reduction of frustration to trust them a lot more.
    - shakezula 4y ago
      > looking at a PR by a dev who has been around for a long time This is precisely the case where I think you need code review the most, because in my experience, time makes people complacent. Code review is peer review, the key agent of the scientific method. I just can’t abide a process that doesn’t include peer review. > Donald Rumsfeld's unknown-unknowns. are an excellent reason to have others question your code. Code review is a culture issue. If you have people who are so unfamiliar with code as to not be able to review it, then that’s a clear knowledge silo that seems perfectly addressable by _code review_. It’s not about the code, it’s about the knowledge transfer and peer enforcement.