3 ms·

I've honestly considered just making a script that hits the checkbox automatically because we have to have "code review" for compliance but 99% of the time I si

by Spivak 2y ago

I've honestly considered just making a script that hits the checkbox automatically because we have to have "code review" for compliance but 99% of the time I simply don't care. Is the change you made gonna bring down prod, no, okay ship it. At some level if I didn't trust you to not write shitty code you wouldn't be on our team. I don't think I want to go all the way and say code review is a smell but not really needing it is what code ownership, good integration tests, and good qa buys you.

pacifika 2y ago
Code review is a good onboarding on upskilling tool, as a reviewer.
rgbrgb 2y ago
How do you bootstrap the trust without code review? Always confused how teams without review onboard someone new.
- ajmurmann 2y ago
  Pair-programming is one option. It might seem expensive at first glance but usually both parties get a lot of value from it (new possible ideas for the teacher if the learner isn't entirely junior and obviously quick learning for the new person)
  - Spivak 2y ago
    We do pair programming for new hires, works well. I'm currently frustrated because our code reviews are both mandatory due to security certifications but also completely worthless to our team structure except Slack spam asking someone to click the approve button. * The work that is done is decided beforehand, the code in every PR corresponds to a card that's already been discussed. * There's no incentive whatsoever to "sneak something in" and if you do so maliciously you'll be fired and maybe prosecuted depending on the damage. * Your code goes through integration testing and QA so you'll never (immediately) take down prod. * I have backups coming out my ears that assume the code running on our app servers is actively malicious so you couldn't cause data loss if you tried. * Norms are all enforced in code, which makes discussions about style pointless. If it passes CI it's good enough.
    - ajmurmann 2y ago
      To play devil's advocate: "* There's no incentive whatsoever to "sneak something in" and if you do so maliciously you'll be fired and maybe prosecuted depending on the damage." How would you discover the snuck in code in a timely fashion?
- criley2 2y ago
  I think you just accept the risk, same as not doing code review. If you're doing something Serious™ then you follow the serious rules to protect your serious business. If it doesn't actually matter that much if prod goes down or you release a critical vulnerability or leak client data or whatever, then you accept the risk. It's cheaper to push out code quickly and cleanup the mess as long as stakeholders and investors are happy.
- deleted 2y ago
  [deleted]
Tainnor 2y ago
> At some level if I didn't trust you to not write shitty code you wouldn't be on our team. Code review isn't about preventing "shitty code" because you don't trust your coworkers. Of course, if 99% of the time you don't care about the code review then it's not going to be an effective process, but that's a self-fulfilling prophecy.

by Spivak 2y ago

pacifika 2y ago

Code review is a good onboarding on upskilling tool, as a reviewer.

rgbrgb 2y ago

How do you bootstrap the trust without code review? Always confused how teams without review onboard someone new.

ajmurmann 2y ago
Pair-programming is one option. It might seem expensive at first glance but usually both parties get a lot of value from it (new possible ideas for the teacher if the learner isn't entirely junior and obviously quick learning for the new person)
- Spivak 2y ago
  We do pair programming for new hires, works well. I'm currently frustrated because our code reviews are both mandatory due to security certifications but also completely worthless to our team structure except Slack spam asking someone to click the approve button. * The work that is done is decided beforehand, the code in every PR corresponds to a card that's already been discussed. * There's no incentive whatsoever to "sneak something in" and if you do so maliciously you'll be fired and maybe prosecuted depending on the damage. * Your code goes through integration testing and QA so you'll never (immediately) take down prod. * I have backups coming out my ears that assume the code running on our app servers is actively malicious so you couldn't cause data loss if you tried. * Norms are all enforced in code, which makes discussions about style pointless. If it passes CI it's good enough.
  - ajmurmann 2y ago
    To play devil's advocate: "* There's no incentive whatsoever to "sneak something in" and if you do so maliciously you'll be fired and maybe prosecuted depending on the damage." How would you discover the snuck in code in a timely fashion?
criley2 2y ago
I think you just accept the risk, same as not doing code review. If you're doing something Serious™ then you follow the serious rules to protect your serious business. If it doesn't actually matter that much if prod goes down or you release a critical vulnerability or leak client data or whatever, then you accept the risk. It's cheaper to push out code quickly and cleanup the mess as long as stakeholders and investors are happy.
deleted 2y ago
[deleted]

Tainnor 2y ago

> At some level if I didn't trust you to not write shitty code you wouldn't be on our team. Code review isn't about preventing "shitty code" because you don't trust your coworkers. Of course, if 99% of the time you don't care about the code review then it's not going to be an effective process, but that's a self-fulfilling prophecy.