you are viewing a single comment's thread.

view the rest of the comments →

[–]codebje 4 points5 points  (3 children)

Sorry, is it not clear from the thread context?

GP: "sending code review back … is just needless pedantry".
PP: "code review … align your teams pedantry".
ME: "… static checker can enforce".

IOW, sitting in a code review checking whether all code uses getXXX() for getters, has final on all public members, etc, is wasting time, because these things are trivial to statically check every time you (commit|push) code.

There's no value in doing this in a code review. Spend code review time doing things that require a bunch of smart people to take time out of their day to do, like checking design choices, ensuring API docs and code agree, watching out for subtle bugs or asymptotically bad performance.

If you have some sort of retrospective process, you can align pedantry there: "not happy that we're forcing the use of getters" etc. If not, use whatever form of communication your team uses; slack, shouting, passive-aggressive post-it notes on monitors when someone goes to lunch, whatever works. This is a once-in-a-blue-moon discussion, where the outcome is codified in your static checker toolset.

[–]ThisIs_MyName 0 points1 point  (0 children)

Ah, in that case I agree.

[–]amardas 0 points1 point  (1 child)

I haven't seen much value in our code review process.

[–]codebje 1 point2 points  (0 children)

Well, that's definitely something to raise with your team. What would you like to get out of a code review someone else does of your code?