3 ms·

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!” approv

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.

StreetChief 3y ago
What if a pr was actual code review and not performative bullshit?
- buster 3y ago
  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.
esafak 3y ago
It only breaks in staging, right? I hope you're not running anything in the physical world like Uber or Tesla.
lmeyerov 3y ago
`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
bootsmann 3y ago
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.
- buster 3y ago
  So you don't do pair programming and you don't have some testing/quality assurance process in place?
marco_patino 3y ago
[dead]

StreetChief 3y ago

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

buster 3y ago
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.

esafak 3y ago

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

lmeyerov 3y ago

`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

bootsmann 3y ago

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.

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

marco_patino 3y ago

[dead]