Hacker News Reader

Comment by beaker52 | Hacker News Reader

Looking at the way GitHub are selling this “feature”, I feel like some of the engineers who are going to be excited about this feature for “reviewability” reasons are, in particular, those who’ve forgotten that they should be splitting changes into multiple logical commits inside a PR. And instead of that they’re now going to use multiple, single commit branches and stack them because stacked PRs are a “new” “feature”.

And the upshot for the LLM providers is that they get to charge for n reviews, instead of one.

View on HN

I think plenty of people explained the difference between stacked PRs and individual commits in 1 PR. You’re not wrong, but it’s just a matter of path of least resistance. There is no way to leave a comment on a particular commit in a PR. Also all commits addressing PR feedback get tucked at the bottom of the commit list. Unless you do some crazy git gymnastics and rewrite the PR history and confuse everyone. With stacked PRs you also (as a maintainer or a reviewer) have the option to merge some and not others. Like here are 3 stacked PRs, one for provisioning some AWS or Azure resources that I’ll need, one for implementing the APIs using those resources, and one for updating the UI to use the new APIs. You can then say “let’s get the 2 backend PRs in and hold off on the UI as we’re changing that entire view”. You can’t do that with multiple commits in a PR without asking the person to redo the PR, then you’re back to git-foo.

Yes, GitHub could have made the UI allow a “per commit” comments somehow, then allow you to select the set of commits to include in the merge somehow, then write a blog post on how to manage “Address PR comments #1” commits. But the stacked PRs solve all that. Not to mention how people treat commits as their own internal save states. I always enable “squash and merge” option because I think it makes a lot more sense to have 1 commit on main per PR where all the context of the change is either in the commit message or the linked PR.

Also LLM providers charge per token. Charging per “work unit” is still not a solved problem. You can’t charge per “review” when your cost is per token. Just like airlines can’t charge “per ticket”, they have to charge differently depending on the destination. Unless you invent some bs arbitrage to lure users and eventually bait and switch on them.

Splitting changes into multiple commits is a much worse experience if you actually have self-contained dependent changes.

1. The whole review interface isn't set up for reviewing individual commits.

2. You can't merge changes progressively.

3. CI doesn't run on each commit.

4. If you have linear history (good idea IMO) you'll lose your nice commit history when you merge it.

This is much better.

You can read each individual commit and review the PR as a whole. This is the way many people design and review PRs.

You don’t need to merge those changes progressively. If you do, you go through exactly the same process of creating a separate branch and PR. The only difference is that GH has now added some UI and automation for rebasing and merging the PRs. In the past we would have explained the chaining in the PR and rebased manually.

You don’t need CI to run on each commit.

You only lose your commit history if you squash merge, many people don’t, and you don’t have to either.

The arguments come from angle that doesn’t appear to be aware that stacked PRs were a thing before GH made these UX improvements.

Reply on news.ycombinator.com