Haha, good luck working with a team with more than 2 people. A good reviewer looks at the end-state and does not care about individual commits. If im curious about a specific change i just look at the blame.

▲

tasuki 8 hours ago | parent | next [-]

> A good reviewer looks at the end-state and does not care about individual commits.

Then I must be a bad reviewer. In a past job, I had a colleague who meticulously crafted his commits - his PRs were a joy to review because I could go commit by commit in logical chunks, rather than wading through a single 3k line diff. I tried to do the same for him and hope I succeeded.

▲

theshrike79 7 hours ago | parent | next [-]

And then someone comments on a thing, they change it and force-push another "clean" history on top and all of your work is wasted because the PR is now completely different =)

▲

mgfist 6 hours ago | parent | prev | next [-]

Why are those not just separate PRs? Or if they really needed to be merged at once - they should still be separate PRs but on a feature branch

▲

seba_dos1 6 hours ago | parent [-]

Why have PRs - groups of commits to pull - then if all you need is a single patch file?

	▲	mgfist 5 hours ago \| parent [-]
		You can, but most of us work in Github and having a PR to dump commits to is very easy and convenient. Then, when you get some feedback on your PR, you can dump more commits and it's very easy for the reviewer to see what has changed since the last time they reviewed it. I feel like what you're arguing is that you should clean up your commits before anyone else sees them. Fair. But you could also clean it up right before merging to main. It's not that different, except the latter is much less annoying, particularly when going back and forth with people. I know this is a very github centric workflow, but that's where most engineers work now, and it's nice and easy. This wouldn't work for eg: contributing to linux, but that's not what most of us do.

▲

KptMarchewa 7 hours ago | parent | prev [-]

Split the PR rather than force me to wade through your commit history. Use graphite or something else that allows you to stack PRs.

	▲	jlokier 4 hours ago \| parent [-]
		Why is it "wade through" if there are 10 clearly distinct but dependent commits, but comfortable if it's 10 stacked PRs instead? They are basically the same thing, presented ever so slightly differently. I think in most teams I've worked with, the majority of developers (> 85%) barely undestand what Git is doing or what things mean inside GitHub, have never seen commit history as a graph, have never run something like "git log --oneline --graph --decorate" or "--format", and have never heard of "git range-diff" which is very useful for following commit/PR/unit changes. Personally I review using "git" itself, so I see the graph structure either way, and there's little difference between stacked PRs, commit chains in a single PR, or even feature branches, from that point of view. Even force-push branch updatea aren't difficult to review, because of the reflog and "git range-diff". The differences are mainly in what kinds of behaviour the web-based tooling promotes in the rest of the team, which does matter, and depends on the team. I agree with you if you're using Graphite instead of GitHub. Having a place to give feedback and/or approval on the individual "units" (commits in a PR, or PRs in a stack) is useful, grouping dependent but distinct changes is useful, and diff'd commit evolution within each unit PR in response to back-end-forth review feedback is useful in some collaborative settings. Though, if you know "git range-diff" and reflog, that shows diff'd commit evolution quite well. In GitHub, people are confused by stacked PRs both conceptually and due to the GitHub UX around them. Most times when I've posted a stacked PR to a GitHub project, other people didn't realise it was stacked, and occasinally someone else has merged the tip of a stack made by me, and been surprised to see all the dependent PRs merged automatically as a side effect. Usually before they get to reviewing those other PRs :-) People understand commit sequences in a PR, though I've rarely seen people treat the individual commits as units for review when using GitHub, unfortunately. In the Linux kernel world where Git was born, the PR flow is completely different from GitHub: Their system tends to result in feedback on individual commits. It also encourages better quality feedback, with less nitpicking, and better quality commits.

▲

jfengel 7 hours ago | parent | prev | next [-]

Sometimes I have to go back and fix a bug that appeared during another branch. Having the original commits helps me bisect it.

Not often, but given that it costs me nothing to have it all in my tree, I'd rather have it than not.

▲

hhjinks 8 hours ago | parent | prev | next [-]

You review code not to verify the actual output of the code, but the code itself. For bugs, for maintainability. Commit hygiene is part of that.

▲

seba_dos1 8 hours ago | parent | prev [-]

I have no troubles working on big FLOSS projects where reviews usually happen at the commit level :)

▲

theshrike79 7 hours ago | parent [-]

So if a PR consists of 20 commits, they review every single commit linearly without looking at the end result first?

	▲	seba_dos1 6 hours ago \| parent [-]
		Yes, and in some projects 20 commits is not even a big PR, more like "regular sized". The LKML's first page is now full of PRs with around 20 commits, here's a random one as an example: https://lore.kernel.org/netdev/20260408121252.2249051-1-dhow... And here's a slightly smaller one which isn't about "miscellaneous fixes": https://lore.kernel.org/netdev/20260408122027.80303-1-xuanzh... Some of these commits even get reviewed by different maintainers before being merged, which is common when a patchset touches several subsystems at once.