Comment by pc86
2 years ago
I'm honestly not familiar with this "I had no choice but to approve" mindset. In the greenfield projects I've worked on there was always at least one person, sometimes several, who had the authority to just say "no, this is far too complex, scratch it all and we'll meet tomorrow to discuss next steps." Sometimes it ended up with breaking the PR into several more manageable pieces, sometimes it ended up with a wholesale rewrite / refactor of that component.
I didn't have enough leverage in that company to say such things and the project had already been running for a couple of years when I joined (not greenfield).
The last greenfield project which I managed from scratch, we didn't have this problem because all developers shared the same mental model of what we were building before we wrote any code. We had a lot of discussions beforehand to get to this shared understanding. There was literally not a single PR which surprised me throughout the entire project and I'm sure none of my PRs were a surprise to any of my team mates either. There was plenty of disagreement throughout but it was always fully resolved through discussion before we started coding each major feature.
In my experience it pops up when company politics get involved, or somebody gets attached to their idea about how certain things should or shouldn't be done. In that case often the safest thing you can do for your own reputation and appearance is to leave some very gentle notes that a certain thing maybe could have been done differently, but approve it anyway on the grounds that it works for now. That way you're not seen as an obstructionist, but if things do go wrong you at least have a written record so you can say "I told you so".