@Root has a code review.

CR comment: “Why would you do it this way? It’s awful. Clean it up!”

Totally fair. I had copied the legendary dev’s code, and it was ick. Cleaning it was easy and enjoyable. I cleaned the source, too.

CR comment: “Why would you touch this? It’s outside the scope of the ticket. You could get it working without changing all this.”

Revert…

CR comment: “The interfaces don’t match. Now it’s confusing, and that makes it harder to maintain.”

🤦🏻‍♀️

Have you found a better place?

@iiii I stopped hearing back from recruiters a few weeks ago, so I’m assuming most companies have a hiring freeze until Jan. That’s also been the norm everywhere I’ve worked.

Idk, sounds like legit comments to me. Neither you nor reviewer knew for sure the spaghetti factor will kick in, and touching unrelated parts of code is a legit MR concern

Maybe it's best to just abandon the PR and close the ticket

@netikras Nah, if you see it, you have to fix it. Maybe in a seperate PR, but it has to be done.

Don't forget: They literally pay you to play this game.

And you actually can answer rhetorical questions. It most often isn't expected - but definitely a legitimate way to get your point across.

The code review game works best if the refined pull request will be reviewed by another teammate who did not read the previous discussion at all.

@Oktokolo separate pr - sure. Absolutely. Log a separate ticket, estimate it, add to sprint so its costs are covered by client [and changes are known by others] and go for it. But mixing multiple fixes under an unrelated pr - that's just asking for confusion

Refer to the other review comment and leg them battle it out

Y’all are missing it.

The existing code allows overrides, and does some work to figure out which overrides go where. It’s functional but ick.

My addition is a very similar feature, so I coped the existing interface. And rightfully was told to clean it up.

I cleaned up both mine and the original, and got chewed at for changing the original’s interface.

I reverted the original, and got chewed at because the new one is different than the original — despite them asking for exactly that.

1) There exists an A.
2) I added a B.
3) Reviewer says capitals are ugly.
4) There now exists: a, b.
5) Reviewer says not to change A.
6) There now exists: A, b.
7) Reviewer is then upset they don’t match.
8) 🤦🏻‍♀️

My solution: there exists now Aa, b. It’s the best compromise between the conflicting demands: all legacy code remains unchanged, patch is smaller, and new code can use the cleaner interface.
> inb4 “Why does this have two interfaces now?!”

Rock and a hard place. There’s no making them happy.

@LotsOfCaffeine They don’t respond to responses on code review comments.

I answer their questions, I ask followup questions, I tag them, etc. No responses. But they sometimes add more comments/complaints later.

If I bug them about it in slack, sometimes they respond, but usually it’s terse and unhelpful, or “‘I’ll get back to you” — and never do.

Is it any wonder I hate this place? ^^

@Root maybe you should go back to A and B and see if they still complain or if it's fine now. In the best case you can spend years in that review loop and harness it as a natural energy source to power up the office building.

@Root I don't think I can blame you for hating it

They’re rejecting good code and bad code. Solution? Don’t write code. Submit a PR with a comment or something lol.

Argh. Always frustrating when this happens. Normally, i love code reviews coz they often make my code better when this back and forth happens, it is always frustrating. My manager is so empathic he would say i know this is annoying but the reason is so and so and that helps with the frustration.

Honestly, I'd just add a "why" comment with snarky remark as "because it seems to confuse some people". Defend your work, man. Don't blindly agree with any CR comment, especially if you know why you did it.