3 ms·
You can be honest with them without bringing up things that aren't relevant. There's a reason you don't like the PR: it's too big, it has a lot of conditional
by tobz 11y ago
You can be honest with them without bringing up things that aren't relevant. There's a reason you don't like the PR: it's too big, it has a lot of conditional logic, etc etc. There's a bit of personal dislike with it, but that dislike is also founded. There's increased complexity, which makes it harder for others to understand, and most likely harder to test. It comes out as a personal feeling of "ughhh" when you look at the code, but there's an objective component to it that, if you take the time to, you should be able to get across.
Everyone is different, and I certainly don't know all of the right words to say, so I'll simply say what I think should be mentioned no matter what: point out the problems with the code, not the person. "What if we broke this apart in two right here? It looks like it'll be hard to test this in isolation otherwise." "What are your thoughts on some sort of pattern matching / switch statement here instead of using if/else blocks? Any downsides? Might end up being a little cleaner."
You're trying to make the case the code can be better, but not that it's horribly bad. I mean, if it works, but it's ugly, you have a base. They got the job done. Now it's time for you to help them make it better.