Hacker News new | ask | show | jobs
by xyzzy123 604 days ago
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.

1 comments

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.
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.