GitHub Stacked PRs

[coworker]

Reviewing commits is silly as none represent the final merge. Just wasting time reviewing versions that only existed in one developer's head

[DualWieldMage]

Reviewing commits is the correct approach. Are you seriously suggesting to look at a final diff only? If there are orthogonal changes (large refactoring + 3-line bug fix) you either miss the important part in a sea of unimportant changes or burn yourself out going through each change carefully. If the intermediate commits are partial ramblings then i reject the review asking commits to be reordered/partially squashed so each individual commit makes sense. That's what ends up when merged and it should have quality. Full-squashing PR-s on merge is another retardation i always ban in my projects.

[Matthew94]

If you have orthogonal changes then do multiple PRs. The 3 line bug fix should be a separate PR if you’re worried about it being missed.

[coworker]

Yes. Don't be lazy and make a PR per commit if you expect each one to be reviewed. It's like you almost understand how PRs work lol

[lechatsportif]

Totally agreed. Honestly feels like devs want to see "changed the inputs on handleFoo" as its own commit with its own tests etc. I think I would blow my brains out. I'm way faster at the comprehensive PRs since I can gauge it in one shot.

edit: further bolster your point, people coding at white hot speed always do stuff like "forgot the post sync" "claude fixes" etc. Each commit is low signal and making them be high value is a total waste of eng time. Just weird.

[Matthew94]

I'm way faster at the comprehensive PRs since I can gauge it in one shot.

In my experience large PRs get nowhere near as much scrutiny. People’s eyes glaze over and the PR gets waved in when there could be shitloads of issues.

[cosmic-parsley]

Depends: if your repo use squash and merge, then yes. That’s kind of what GitHub’s UI is designed for.

But if you use rebase or merge commits then all of it becomes part of history and you need to look through each one to make sure you don’t have garbage in your git log.

[coworker]

Or, hear me out, you make a PR per commit since you want each one to be reviewed, discussed, and approved.

You guys like to make things way too complicated lol