More often than not this is “it seems like there are problems with how it’s implemented but I can’t think of them now” but sometimes it definitely is “that’s not the way I would have done it and therefore wrong”
All that said, I used to work with a guy that would find a reason to fail every code review. It became common for people to add intentional easy mistakes so that the lead could feel like he was contributing.
Oof, so counterproductive. I’m a hard reviewer, always try to hold others to the standard of code I’d like to work in, and be held to myself, but every once in a while I see a PR that’s just… no changes required.
I love just hitting accept without making any feedback, it means my coworker valued my feedback and actually internalized it. Trying to laser in and nitpick something unnecessary would be a waste of all our time.
It could be a legitimate criticism. Sticking to a codebase style and to the language idioms improves clarity and thus maintainability. But then, that means the way to do it should already be clear.
Agreed, and those patterns often have reasons to them, which may not be obvious at first glance, and are often forgotten when your spend a long time with them.
I’m not a programmer, but I work in a creative field. I routinely have to create red herrings so the producer, creative director, and clients can feel like they contributed by suggesting to remove the thing I intentionally put in there just so they can suggest its removal. It’s exhausting trying to manage everyone’s egos.
More often than not this is “it seems like there are problems with how it’s implemented but I can’t think of them now” but sometimes it definitely is “that’s not the way I would have done it and therefore wrong”
All that said, I used to work with a guy that would find a reason to fail every code review. It became common for people to add intentional easy mistakes so that the lead could feel like he was contributing.
Oof, so counterproductive. I’m a hard reviewer, always try to hold others to the standard of code I’d like to work in, and be held to myself, but every once in a while I see a PR that’s just… no changes required.
I love just hitting accept without making any feedback, it means my coworker valued my feedback and actually internalized it. Trying to laser in and nitpick something unnecessary would be a waste of all our time.
It could be a legitimate criticism. Sticking to a codebase style and to the language idioms improves clarity and thus maintainability. But then, that means the way to do it should already be clear.
Agreed, and those patterns often have reasons to them, which may not be obvious at first glance, and are often forgotten when your spend a long time with them.
I’m not a programmer, but I work in a creative field. I routinely have to create red herrings so the producer, creative director, and clients can feel like they contributed by suggesting to remove the thing I intentionally put in there just so they can suggest its removal. It’s exhausting trying to manage everyone’s egos.