Hacker News Reader

Comment by IshKebab | Hacker News Reader

IMO there's no point having a clean history of commits within a PR. With rare exceptions, if you have a PR with a clean history of commits and each commit compiles and passes the tests... they should be separate PRs! If it isn't clean then it should be squashed.

A few exceptions:

1. When refactoring often your PR is "do an enormous search and replace, and then fix some stuff manually". In that case it's way easier to review if the mechanical stuff is in a separate commit.

2. Similarly when renaming and editing files, Git tracks it better if you do it in two commits.

3. Sometimes you genuinely have a big branch that's lasted months and has been worked on by many people and it's worth preserving history.

Also I really really wish GitHub had proper support for stacked PRs.

View on HN

This is truer now that `git bisect --first-parent` exists. But it didn't always. And even then, there are times you find out that there is "prep work" to land your feature. And a PR just to do some deck chair moving that makes a follow-up commit easier is kind of useless. I have done prep work as a separate PR, but this is usually when it is more extensive than the feature and it is worthwhile on its own.

Another instance is a build system rewrite. There was a (short) story of the new system itself and then a commit per module on top of that. It landed as 300+ commits in a single PR. And it got rebased 2-3 times a week to try and keep up as more bits were migrated (and new infra added for things other bits needed). Partial landing would have been useless and "rewrite the build system" would have been utter hell for both me developing and anyone that tries to blame across it if it hadn't been split up at least that much.

Basically, as with many things in software development, there are no black-and-white answers here.

> IMO there's no point having a clean history of commits within a PR. With rare exceptions, if you have a PR with a clean history of commits and each commit compiles and passes the tests... they should be separate PRs! If it isn't clean then it should be squashed.

I think that whether clean history has a point, really depends on how deep are you refinement sessions. And perhaps a bit on the general health of your codebase.

If you don't do refinement with your editors open and grind tickets into dust, there will be side-changes adjacent to each PR which are not directly related to the ticket. These are better to have their own commit (and commit message).

A perfect illustration of a backwards mindset. If this made sense then the standard or least common denominator PR tool would work better with many small PRs, which here also means they must be able to depend on each other. (separate PRs!!!) So is it?

> Also I really really wish GitHub had proper support for stacked PRs.

No. It doesn’t even support it.

So how does this make sense? This culture of people wanting “one PR” for each change, and then standard PR tool that everyone knows of doesn’t even support it? What’s the allegiance even to, here? Phabricator or whatever the “stacked” tools are?

It’s impressive that Git forge culture has managed to obfuscate the actual units of change so much that heavyweight PRs have become the obvious—they should be separate PRs!—unit of change... when they don’t even support one-change-then-another-one.

Yeah it's kind of infuriating really. It's not like it's an uncommon workflow either. Everywhere I've worked people end up with PRs that just say "this depends on this other PR; ignore the first commit".

Gitlab kind of supports it - if your second PR's target branch is the first PR then it will only show you the code from the second PR and it will automatically update the target branch to master when the first one gets merged. I wouldn't say it's first class support though.

Sapling sort of has support for making it work on GitHub: https://sapling-scm.com/docs/addons/reviewstack/

And there was some forge that supports Jujutsu that has proper first class support, but I can't find it now.

Anyway it's a very useful workflow that lots of people want and kind of insane that it isn't well supported by GitHub.

To be fair I can't remember the last time GitHub introduced any really new features. It's basically in maintenance mode.

Reply on news.ycombinator.com