3 ms·

Just out of morbid curiosity... have you actually experienced multiple 'seniors' giving conflicting code review comments about code style (of all things)? That

Just out of morbid curiosity... have you actually experienced multiple 'seniors' giving conflicting code review comments about code style (of all things)? That sounds quite dysfunctional. (EDIT: Sure, nitpicks may differ, but...)

ses1984 6y ago
Style yes, formatting no. Worked with two developers who endlessly argued whether or not we should handle a certain bit of complexity in a certain layer or the next layer over, so we ended up handling it in both layers with the downsides of both.
rumanator 6y ago
> That sounds quite dysfunctional. No,it doesn't. It sounds like the expected outcome of not enforcing an established style with automated tools. All it takes is someone posting a merge request with a bracket out of place, or tabs instead of spaces which screws layout because yes IDEs have custom definitions and a dude happened to have opened a source file with an editor that wasn't properly configured. Boom, merge request receives two comments pointing out the bracket and how indentation is off. Congrats, about 20 minutes of your team's day are wasted because that's the time it takes to receive feedback from the merge request, be briefed on the remarks, go through the code and fix whitespaces, commit your change, push those changes, update the merge request, and wait for a team member to review your update. No drama. No dysfunctional team. No disagreement, even. But those 20 minutes of your life are lost forever.
- Quekid5 6y ago
  > It sounds like the expected outcome of not enforcing an established style with automated tools. Unfortunately those also have significant downsides around large-scale refactoring. What I always do (and advise other code reviewers to do) is to just ask themselves: Does this code follow the local style in the file being edited? That simplifies things greatly, IME.

ses1984 6y ago

Style yes, formatting no. Worked with two developers who endlessly argued whether or not we should handle a certain bit of complexity in a certain layer or the next layer over, so we ended up handling it in both layers with the downsides of both.

rumanator 6y ago

> That sounds quite dysfunctional. No,it doesn't. It sounds like the expected outcome of not enforcing an established style with automated tools. All it takes is someone posting a merge request with a bracket out of place, or tabs instead of spaces which screws layout because yes IDEs have custom definitions and a dude happened to have opened a source file with an editor that wasn't properly configured. Boom, merge request receives two comments pointing out the bracket and how indentation is off. Congrats, about 20 minutes of your team's day are wasted because that's the time it takes to receive feedback from the merge request, be briefed on the remarks, go through the code and fix whitespaces, commit your change, push those changes, update the merge request, and wait for a team member to review your update. No drama. No dysfunctional team. No disagreement, even. But those 20 minutes of your life are lost forever.

Quekid5 6y ago
> It sounds like the expected outcome of not enforcing an established style with automated tools. Unfortunately those also have significant downsides around large-scale refactoring. What I always do (and advise other code reviewers to do) is to just ask themselves: Does this code follow the local style in the file being edited? That simplifies things greatly, IME.