4 ms·

Then the reviewer should make that clear. And often that happens. My objection is to the gatekeeping self-appointed code hero that has some arbitrary plan. One

Then the reviewer should make that clear. And often that happens. My objection is to the gatekeeping self-appointed code hero that has some arbitrary plan. One they probably don't adhere to themselves, but insist everybody else toes the line. Don't free objects that way! Don't test for null like that! I know a better way I heard of in a blog or a meme or on a message board, and I will delay the project and burn runway to make myself feel important!

nradov 5y ago
This is why competent organizations have written coding style guidelines. Everyone's code should look the same, and personal preferences shouldn't be allowed for things like freeing objects or testing for null. When the committer and reviewer can't reach consensus between themselves then they can always escalate to the program technical lead for a final decision.
- JoeAltmaier 5y ago
  Now, see, that worries me right there. Because its just something a code-review obstructionist would say. They probably wrote the guidlines too. It doesn't matter how you do those simple things. Almost always. If a case arrises where it does matter and somebody uses the wrong one, then by all means bring it up. Otherwise, hit Approve and move on.
  - nradov 5y ago
    Enforcing consistent compliance with the guidelines starts to matter a lot in large organizations where code lifetimes are measured in decades. When you see a commit that's out of compliance on the details that's usually a sign of deeper, more serious quality problems. Working on code bases like that isn't for everyone. Some engineers are better suited to less demanding roles where the little details don't matter as much.
    - JoeAltmaier 5y ago
      Sorry, didn't expect that to sound like such an ad-hominem. How about: the attitude that the code appearance trumps the project goals, is central to the confusion that breaks code review?

nradov 5y ago

This is why competent organizations have written coding style guidelines. Everyone's code should look the same, and personal preferences shouldn't be allowed for things like freeing objects or testing for null. When the committer and reviewer can't reach consensus between themselves then they can always escalate to the program technical lead for a final decision.

JoeAltmaier 5y ago
Now, see, that worries me right there. Because its just something a code-review obstructionist would say. They probably wrote the guidlines too. It doesn't matter how you do those simple things. Almost always. If a case arrises where it does matter and somebody uses the wrong one, then by all means bring it up. Otherwise, hit Approve and move on.
- nradov 5y ago
  Enforcing consistent compliance with the guidelines starts to matter a lot in large organizations where code lifetimes are measured in decades. When you see a commit that's out of compliance on the details that's usually a sign of deeper, more serious quality problems. Working on code bases like that isn't for everyone. Some engineers are better suited to less demanding roles where the little details don't matter as much.
  - JoeAltmaier 5y ago
    Sorry, didn't expect that to sound like such an ad-hominem. How about: the attitude that the code appearance trumps the project goals, is central to the confusion that breaks code review?