Re:Google takes the pain out of code reviews, with 97% dev s...
by withinboredom on Monday December 04, 2023 @05:22PM
(#38524017)
To be clear, I'm not talking about code style (easily objectively quantifiable) or bad code (also easily objectively quantifiable), but architecture/design. This can be things like which folder an interface belongs in, where to put a method in a parent/child relationship that touches both, when to use value objects vs. scalars, when to break functionality into another service/library, which methods are allowed to call which methods, which classes must be injected vs. instantiated, etc.
standard is relative, and while I'm talking generically as someone on the internet with no context of your experience, a code reviewer shouldn't. Someone literally saying "the code is not up to standards" needs to clarify.
For my generic take, take "standard" as "whatever that team has laid out in its code architecture". Sometimes there may be times to question the standards if it compromises other important factors, but personally I don't think the code review is the time to fight over whitespace and bracket placement (those should be solved via a linter anyway). Make the correction and submit, those later discussions can happen offline.
>A code review simply isn’t the place to enforce your unique perspectives on someone else.
Likewise, the goal is to align everybody, not to argue over semantics or enforce your own POV. That's why code style sheets are a thing; it's a mediator that can serve as its own battlground should team/company style need to be changed. Again, every place is different, but
1. style should be automated as much as possible. If there's some stupid whitespace rule, it should be a one button click on my IDE from some provided linter to resolve (even if I will proceed to re-lint it to my style afterwards on my local machine).
2. style sheets can have suggestions as well as laws. Be reasonable on what is what
>For some people, they want layers.
sure, lawful evils exist, both maliciously and inadvertently. Sounds like you ran into both kinds. Office politics are inescapable even at the best companies.
That sounds like another discussion to adjust to style sheet, not throw it out upright. If you see a bunch of examples of a law being broken, adjust it to a suggestion unless is a strong argument made otherwise. (most) companies aren't a congress where amending such things takes months of proposal, and we can very much adjust the document to the people if there's no resistance.
Now, do you actually care enough to spearhead that change? I imagine many don't, and there in lies the problem. That's why I'm not on management track; I don't have the care nor attention to worry about documenting such things unless someone throws it at me. I'll leave that to those who do care.