5 ms·

I do this in code occasionally. Leave in a mistake and see if the reviewer catches it. Now I know who pays attention and who rubber stamps my diffs. Sometimes y

I do this in code occasionally. Leave in a mistake and see if the reviewer catches it. Now I know who pays attention and who rubber stamps my diffs. Sometimes you want one kind of review or the other.

freeqaz 6y ago
You have to A/B test this though because who knows when somebody is in crunch and doesn't have time! Also, from a teammate perspective, it might be worth pointing it out to the person. "I noticed a bug in the PR that I found later... want to show you why it could be a problem so we don't let it slip next time" Imo I usually just fire the PR off and message on Slack saying, "this one needs to ship ASAP, anything truly bjork'd here?" Or if it's going to be critical code, "The blast radius on this one is high. Give it a thorough scan please!" Communication is key :)
- thebean11 6y ago
  > I noticed a bug in the PR that I found later... want to show you why it could be a problem so we don't let it slip next time I don't know, that seems kind of confrontational. If I got called out for missing a bug in a review I'd be annoyed
  - freeqaz 6y ago
    That's true, and it definitely can be! In my opinion, the best way to approach this situation is with "non-violent communication"[0] in mind. Don't make the conversation about "this is your fault, do better", instead make it about "I noticed a bug in my code that could have broken production and I think we can do better". It takes trust to pull off conversations like these without coming across as a prick. This type of empathy is particularly important when mentoring others because... People are always going to make mistakes. They create great opportunities to help somebody learn though. When I am being taught something, I learn much more quickly when somebody points to somewhere I failed versus explaining an "abstract" bad pattern. Maybe having the "abstract" idea first helps. It might make the "failure" easier to stomach later. For me though, those moments of stumbling always stuck as lessons I reflect back on the most. My attention to detail with code became a lot higher when I was on-call and prod broke at 4am because of a bug I let slip! 0: https://en.wikipedia.org/wiki/Nonviolent_Communication#Four_components https://en.wikipedia.org/wiki/Nonviolent_Communication#Four_...
kerbs 6y ago
Not every reviewer has a "fine-toothed comb" philosophy. Unless code is algorithmic in nature, I look at architecture and approach only. A good post on the topic: https://blog.danlew.net/2021/02/23/stop-nitpicking-in-code-reviews/ https://blog.danlew.net/2021/02/23/stop-nitpicking-in-code-r...
roter 6y ago
Kind of a corollary: Place something fixable in your PhD thesis so the examiners can find something to write about. Something that doesn't make you look stupid but will make them feel like they'd done a thorough review and won't focus on your more subtle but perhaps more disastrous errors. Perhaps just a thing PhDs tell other PhDs to do but never actually do themselves.
- r00fus 6y ago
  Isn't this more of a corollary "remove the duck" principle [1]? The idea being, you expect the reviewer to force some change, so put in a trivial & obvious stylistic difference so their need to make changes are assuaged. [1] https://news.ycombinator.com/item?id=9137736 https://news.ycombinator.com/item?id=9137736
- throwaway0a5e 6y ago
  I maintain a small fleet of vehicles. I keep a set of worn pads, rotors and tires that I put on every vehicle before it goes for state inspection. Steering/suspension parts replacement fell off a cliff once I started doing this. I would say this approach taught me something but it just confirmed things I already knew.
  - nullserver 6y ago
    So needed to something to correct? So you made obvious small issues? Used to do this on mock-ups of apps I was building. They wouldn’t fuss at the core functionality if they changed something obvious and not important.
    - throwaway0a5e 6y ago
      >So needed to something to correct? So you made obvious small issues? Exactly. The tall nail gets the hammer. The shop that passes all the shitboxes gets the audit. They don't want that audit and if that audit comes they want the paper trail to make them look like they're hard-asses about everything just like the auditor wants to see so they don't lose their license. The deal is that safety inspections are basically guaranteed work for the shops and exchange they get an incentive to not fudge emissions inspections. I dunno if that's still how they present it but that's what the 3rd party that runs the state training was telling everyone years back and the rules haven't changed since then (other than some more increased reporting requirements that are either to reduce odometer fraud or lay the groundwork for a mileage tax depending on who you ask).
paxys 6y ago
Code reviews aren't there to catch mistakes. That's the job of unit and other tests.
- edflsafoiewq 6y ago
  Then what are they for?
  - yourself92 6y ago
    Design considerations
- worik 6y ago
  Right. So ignore mistakes in code reviews? Unit tests are useful, invariant testing in code more useful, but code reviews and user testing are important too, for catching mistakes.
- ravedave5 6y ago
  Apparently developers spring fully trained from Zeuses head in your company :).
mmmBacon 6y ago
This seems like a strange thing to do.
- greenshackle2 6y ago
  Laying down traps for your co-workers sounds strange to me as well, we must be working in different kinds work environments than the parent commenter.

freeqaz 6y ago

You have to A/B test this though because who knows when somebody is in crunch and doesn't have time! Also, from a teammate perspective, it might be worth pointing it out to the person. "I noticed a bug in the PR that I found later... want to show you why it could be a problem so we don't let it slip next time" Imo I usually just fire the PR off and message on Slack saying, "this one needs to ship ASAP, anything truly bjork'd here?" Or if it's going to be critical code, "The blast radius on this one is high. Give it a thorough scan please!" Communication is key :)

thebean11 6y ago
> I noticed a bug in the PR that I found later... want to show you why it could be a problem so we don't let it slip next time I don't know, that seems kind of confrontational. If I got called out for missing a bug in a review I'd be annoyed
- freeqaz 6y ago
  That's true, and it definitely can be! In my opinion, the best way to approach this situation is with "non-violent communication"[0] in mind. Don't make the conversation about "this is your fault, do better", instead make it about "I noticed a bug in my code that could have broken production and I think we can do better". It takes trust to pull off conversations like these without coming across as a prick. This type of empathy is particularly important when mentoring others because... People are always going to make mistakes. They create great opportunities to help somebody learn though. When I am being taught something, I learn much more quickly when somebody points to somewhere I failed versus explaining an "abstract" bad pattern. Maybe having the "abstract" idea first helps. It might make the "failure" easier to stomach later. For me though, those moments of stumbling always stuck as lessons I reflect back on the most. My attention to detail with code became a lot higher when I was on-call and prod broke at 4am because of a bug I let slip! 0: https://en.wikipedia.org/wiki/Nonviolent_Communication#Four_components https://en.wikipedia.org/wiki/Nonviolent_Communication#Four_...

kerbs 6y ago

Not every reviewer has a "fine-toothed comb" philosophy. Unless code is algorithmic in nature, I look at architecture and approach only. A good post on the topic: https://blog.danlew.net/2021/02/23/stop-nitpicking-in-code-reviews/ https://blog.danlew.net/2021/02/23/stop-nitpicking-in-code-r...

roter 6y ago

Kind of a corollary: Place something fixable in your PhD thesis so the examiners can find something to write about. Something that doesn't make you look stupid but will make them feel like they'd done a thorough review and won't focus on your more subtle but perhaps more disastrous errors. Perhaps just a thing PhDs tell other PhDs to do but never actually do themselves.

r00fus 6y ago
Isn't this more of a corollary "remove the duck" principle [1]? The idea being, you expect the reviewer to force some change, so put in a trivial & obvious stylistic difference so their need to make changes are assuaged. [1] https://news.ycombinator.com/item?id=9137736 https://news.ycombinator.com/item?id=9137736
throwaway0a5e 6y ago
I maintain a small fleet of vehicles. I keep a set of worn pads, rotors and tires that I put on every vehicle before it goes for state inspection. Steering/suspension parts replacement fell off a cliff once I started doing this. I would say this approach taught me something but it just confirmed things I already knew.
- nullserver 6y ago
  So needed to something to correct? So you made obvious small issues? Used to do this on mock-ups of apps I was building. They wouldn’t fuss at the core functionality if they changed something obvious and not important.
  - throwaway0a5e 6y ago
    >So needed to something to correct? So you made obvious small issues? Exactly. The tall nail gets the hammer. The shop that passes all the shitboxes gets the audit. They don't want that audit and if that audit comes they want the paper trail to make them look like they're hard-asses about everything just like the auditor wants to see so they don't lose their license. The deal is that safety inspections are basically guaranteed work for the shops and exchange they get an incentive to not fudge emissions inspections. I dunno if that's still how they present it but that's what the 3rd party that runs the state training was telling everyone years back and the rules haven't changed since then (other than some more increased reporting requirements that are either to reduce odometer fraud or lay the groundwork for a mileage tax depending on who you ask).

paxys 6y ago

Code reviews aren't there to catch mistakes. That's the job of unit and other tests.

edflsafoiewq 6y ago
Then what are they for?
- yourself92 6y ago
  Design considerations
worik 6y ago
Right. So ignore mistakes in code reviews? Unit tests are useful, invariant testing in code more useful, but code reviews and user testing are important too, for catching mistakes.
ravedave5 6y ago
Apparently developers spring fully trained from Zeuses head in your company :).

mmmBacon 6y ago

This seems like a strange thing to do.

greenshackle2 6y ago
Laying down traps for your co-workers sounds strange to me as well, we must be working in different kinds work environments than the parent commenter.