[korijn]

No code changes should be merged purely because the author got less code merged than someone else on their team. This sounds harsh maybe, but by your reasoning, your feelings could end up being the cause for bugs, poor quality code and/or bad design ending up in a release.

Of course, you and your team should work together to _avoid_ having to reject work! But it can and will happen, it's perfectly normal for mistakes to be made, it's how all humans learn.

Trying to deny that people sometimes fail is foolish. Punishing yourself for making a mistake is on you.

The only unfair thing here is taking others in the team hostage with the idea that you are entitled to getting your work merged regardless of its quality, purely because it would make you peeved, cranky, annoyed! That constitutes toxic behavior. If this is a pattern for you, people will avoid working with you.

Instead: embrace the opportunity to learn. Get feedback, reflect with the team, do better next time. Maybe pair up to refactor your work. Take the positive approach!

[camgunz]

I feel like my little comment here became something of a Rorschach test. I'm definitely not saying we should merge bad code to keep merge rates even. All I'm saying is:

- Someone says "build this thing"

- I build "this thing"

- That someone says "just kidding, we're not gonna use it"

- I'm peeved

Someone else in this thread is arguing this is an entitled position, and here you're arguing that... well, I think you're arguing that I think all my code is always amazing and should always be merged.

I'm not! Like I wrote elsewhere I've written some pretty shit code, I've built the wrong thing, and I've built broken things. I'm sure this is true for most SWEs. This isn't the scenario I'm describing.

But I think this discussion has some merit in terms of how we navigate code review. For example, conversely, I've been on the other end of some pretty... bad feedback. The first example that comes to mind is that we had a portal where you could search by text or category, but once you selected a result we wouldn't save your search anywhere (query params, session storage, etc.). Consequently, when you clicked our "back" link, your search would be gone. We YAGNI'd it for a long time, but we accepted a very tight deadline project (COVID/government related) that required a category that needed to be sticky.

I built this using query params, like pretty much every search out there (for good reason). This ended up changing a lot of templates, a couple of front-end React components, and required extra logic in a couple Django controllers. It was a big-ish change, maybe (to my recollection) 300-400 lines across a few stacked PRs--meticulously, for ease of review. All previous tests passed, all the new tests I wrote (typically >= 50% of my PRs were new tests) passed, I even built a punch list of UI tests I ran through (this was going to be a big user-facing feature and I wanted it to be bulletproof). This took I think... 2 days of constant work, so something like ~30 hours.

This wasn't our typical process; we skipped our usual engineering meetings about implementation strategy and what-not. Our team was small--4 people including our CTO--but even so we had a wide diversity of opinion when it came to implementation, architecture, and style, so it kind of ended up being the case that if we wanted anything to get through PR we had to hash it out beforehand. But we literally had 7 days or something to do this, so we just didn't have time.

But, predictably, despite all my tests and punch list, my PRs were rejected as "too much code", and we missed our deadline. We launched without the feature. Our CTO reviewed the vast majority of our PRs, he reviewed these and he was pretty furious about the scope of the changes, blaming me for missing the deadline.

Afterwards, he tried reimplementing it using query params in less code, but failed. He then tried reimplementing it using local storage, which was less code, but had multiple problems: local storage works across tabs which is deeply weird, but even if he switched to session storage, it didn't work in lots of versions of mobile Safari if you're in private browsing mode. I rejected that PR for those reasons, which we disagreed vehemently about. Eventually, a couple months later, we paired on it, and basically reimplemented my work together.

There are obviously a lot of flags in this little story, but I don't think they're wildly out of the ordinary for a startup (if anything, it's way too much process for a 10 person company). My point is that, while I'm sure there are a lot of cases of "I'm God's gift to this company merge all my work never question me" out there, there are also a lot of cases of "no PR is fit to merge the first time" and "I'm a great programmer, you didn't do this the way I would, therefore this isn't good enough" as well.