[xyzzy123]

I think this comes down to a lack of shared understanding on what a PR approval means, it sounds like you didn't believe it meant the same thing as your team did. There's not one standard meaning of the "rubber stamp". It's a culture thing and teams should discuss and eventually agree on what they think it means. Mature teams will have guides, checklists and documented expectations.

It's moderately likely that behind the scenes someone was complaining to your manager that you were blocking their work and that the "metrics" discussion was just a cover for that.

Usually when I review a PR I am just sanity checking the overall approach, figuring out or asking how they tested it, and making sure there's nothing crazy that will cause pain for everyone else later. Detail correctness I don't consider my problem because there's no economic way I can verify it. I can usually approve in minutes and I don't consider it a waste of time, I did the things the process needed of me.

Unless something irreversible is happening (and it should be reviewer's job to be aware of that), the fix for a bad PR is more PRs.

There are projects where almost the only thing that matters is approving quickly because this will let your co-workers get on with their job, this culture tends to evolve in orgs with lots of related repos where you need 5 MRs and pipelines (that flow on to each other) to deploy the tiniest unit change. It's completely dysfunctional but it happens a lot.

I imagine there are places where reviewers are expected to spend an hour "raising the bar" on every PR but I've never worked at one. I'm also not sure if I'd want to.

Note that there's not one thing or process that makes sense, it's very context dependent with lots of exceptions. For example, if someone from a "far away" team is contributing to a particular repo for the first time I will probably reach out to them to see what they're trying to do and review more carefully because they likely have limited context vs a core contributor.

[reviewmoreprs]

Well then why does the PR template used at the company require manual validation steps on every PR if it's not expected that reviewers do them? Do you really think 5 seconds was enough time to "sanity check" the PR? Do you think 5 seconds was enough time to even click the code tab? Why are two "reviews" even required if the reviewers are actually expected to simply rubber stamp every single PR that comes through?

Edit: "the fix for a bad PR is more PRs" has got the be the worst take I've seen in years.

[xyzzy123]

The review requirement often comes from compliance, many standards require a control to stop developers getting code into production by themselves without anyone else looking at it. Sometimes it's because an engineering manager read a book that said it was "best practice" or because "that's what we did at $lastjob". Sometimes it's because someone set up the SCM to require it and never looked at it again. Maybe it made sense to someone 5 years ago but it doesn't make sense now, for the teams and processes you have. It's worth asking and finding out!

I agree 5 seconds is not enough for even cursory review, why even bother, unless it's a compliance thing. In a functional team the thing you do is raise this in retro and discuss what everyone expects and wants from reviewers.

[reviewmoreprs]

If your take is that blindly approving merges with 0 care is actually a good thing and that I "lack understanding" then I simply have to disagree lol. Good luck with that.

[xyzzy123]

Sorry that I was not clear, my bad, I did not mean to imply that you personally "lack understanding" in any way.

What I should have written was that there seemed to be a lack of shared understanding among the team, (i.e, agreement) on the value, meaning and expected process for PR reviews.

I don't think there is any particular level of care that makes sense in all cases for PR reviews, I believe it depends on industry, criticality, the particular repo and who is doing what. I think the most important thing is that everyone is on the same page about what's expected.