alt Hacker NewsI know the aggravation of getting a hairball of code to review, but I often hold my nose. At least find a better reason to send it back, like a specific bug.
If you're sure cyclomatic complexity should be minimized, I think you should put such rules in a pre-commit hook or something that runs before a reviewer ever sees the code. You should only have to help with that if someone can't figure out how to make it pass.
If you're not willing or politically able to implement that, you might be wasting time on your personal taste that the team doesn't agree with. Personally I'm pretty skeptical of cyclomatic complexity's usefulness as a metric.
Replies
I always approve a change with comments for nits that are optional to address. I only hold back approval if there is a legitimate flaw of some sort. Generally this leads to small changes almost always getting approved on the first shot, but larger changes needing at least one back and forth. AI code review tools make it much easier to spot legitimate problems these days.
I just used it here to approximately convey the scale.
the original function was full of mutable state (not required), full of special cases (not required), full of extra return statements (not required). Also had some private helper methods that were mocked in the tests (!!!).
All of this just for a "pure" function. Just immutable object in - immutable object out.
and yes, he was a junior.