Hacker News Reader

Comment by hipadev23 | Hacker News Reader

Super common. I’m not sure how that’s any different than teams who create a PR, beg someone to review it, they glance it for 3 sounds and leave a “LGTM!” approval so you can finally merge to main.

Same result, whole lot less performative bullshit.

We use branches and reviews for large changes. Small scope changes go directly to main. If it breaks, we rollback and tell that person to stop breaking things.

View on HN

`lgtm` is a legit typical outcome. biasing to approving, in many companies & software setups, is a great default. ex: we might `lgtm`, or make comments that are suggestions for follow-on PRs without blocking the current.

However... it's also incredibly hard to self-edit and accurately `lgtm` yourself

4 eye principle brings a lot of redundancy. Even if someone just goes lgtm, it prevents people from pushing a change that no other person has seen which means if the original engineer is on holiday you still have someone who is somewhat familiar with the code.

So you don't do pair programming and you don't have some testing/quality assurance process in place?

What if a pr was actual code review and not performative bullshit?

That doesn't mean you have to block the merge to main. You can do code reviews on the trunk, you can do it on already merged PRs and you can block on the PR until the code review is done. The latter brings a lot of downsides, which we as developers seem to be taking for granted. It looks like developers can't even imagine not working with Github style pull requests.

It depends on your project, but i prefer having a coherent view on all changes on one branch, instead of 20 branches and one outdated trunk.

It only breaks in staging, right? I hope you're not running anything in the physical world like Uber or Tesla.

Reply on news.ycombinator.com