Comment by WorldMaker | Hacker News Reader

Comment by WorldMaker | Hacker News Reader

I wish GitHub's PR UI was better at walking through a PR one commit at a time. As someone who does try to make good commits, and as someone who does try to read PRs sometimes a commit at a time, GitHub's UI gets in the way and keeps trying to drive you back to "whole PR" reviews.

It is very telling in the article itself there is a screenshot of the commits tab in the PR workflow that many don't realize even exists and/or never think to use.

In the Files tab the commit picker has gotten better in recent years, but it is still overly focused on selecting ranges of commits over individual ones, and there's no shortcuts to easily jump Next or Previous commit, you have to remember your place and interact with the full pulldown every time. Also, it's hard to read the full descriptions of commits in the Files view and I find I often have to interrupt my flow to open the commit in another browser tab or flip back and forth between the Commits tab and the Files tab in the PR. The Commits tab also defaults to hiding most of the commit descriptions, so it's still not a particularly great reading experience.

It feels like a bit of a bad feedback loop that GitHub's UI doesn't make commit-by-commit reviewing clean/easy because GitHub themselves don't expect most developers to write good commits, but a lot of developers don't write good commits today simply because GitHub's PR interface is bad at reviewing individual commits and developers don't see as much of a point in it if they aren't going to be reviewed in that way.

Graphite addressees this issue in a different way - it lets the author split a pr into several smaller prs, so one can review it piece by piece, but still see the whole super-pr in one place. It does a decent job rebasing stacked prs as well.

If faced with this issue, I would probably just pull the remote/branch locally and step through it, commit by commit, using my preferred Git manager (Lazygit).

My big complaints are with the default experience in how that affects everyone's expectaions, but I do this sometimes, yes. The VS Code "GitHub Pull Requests and Issues" extension has gotten really good and gives a lot of tools for this. If using github.com (and not GHE) you can even use the "." shortcut in any PR to open that PR in github.dev, which is a web version of VS Code (that you can Settings Sync) to review the PR there without even needing the local checkout. But also that's a bit of a "hidden shortcut" and very few developers know about it, which again gets back to what the GitHub interface provides by default and how it makes things discoverable being something of an underlying issue.

Can you describe a little more about why you walk through individual commits instead of just reviewing the latest commit only?

Because the idea is that each commit that you submit is a coherent unit of work. That's why we have commit messages.

Good commits can tell a story. (The article here discusses this, too, and suggests that good commits should tell a story.) When a commit author takes the time to fill out the commit message, it will include things like the "how" and "why" of each step, what they were thinking about as they worked on that part of the whole PR. Often I find that will save you from asking questions like "Why did you take this approach?" which I see all the time in PRs (and make all the time in PRs with "bad" or just "save point" commits).

The PR should be the smallest unit of integration (this works and builds and is ready to merge to the next steps), but the commit is the smallest unit of any progress at all. Ideas don't come fully formed and ready to compile. Progress sometimes includes back tracks and experiments. Good commits say things "hey, I learned from this thing that wasn't working and that's what pushed me into this next direction". They document the journey, why a specific path was taken, what obstacles were in the way, what other paths were explored and dismissed.

Some PR authors can capture a lot of that in a PR description as well, but commits tie that to specific context of the code at a point in time in ways that a PR description often can't, without linking to commits (in which case the commits again speak for themselves).

But yes, not everyone can or will write good commits. I see that partly as a tooling failure because our tools themselves like GitHub PRs don't encourage it, often fail to reward it. I've seen plenty PRs full of commits named only "WIP" and "Fix" and "oops", but the best PRs tell a story in the commits, have meaningful descriptions on each commit. I would love for our tools to encourage more of those because I think they are better PRs; often easier to review PRs and enough good PRs like that form a string of documentation that you can search through (through git blame and git log if nothing else, but that's still a lot of useful research data). (If you keep that data, I know a lot of people like squash merges because they distrust the git DAG and how many tools show ugly or hard to read "subway diagrams" for it instead of simpler collapsible views. But that's another long conversation.)

> But yes, not everyone can or will write good commits.

some people treat commits as meaningful units of independent review, and some people treat them as savepoints and the PR as the only meaningful unit of review, it's a distinction of process, not purity -- both approaches are totally fine, one is not better than the other

git and commits and prs are means, not ends

That's exactly it. What happens in a private branch is an implementation detail and reflects personal work style. Policing that is counterproductive.

No one said anything about "policing" anything. I'm not telling anyone how to write PRs, I'm just suggesting that if we had better tools we'd get "better" PRs for values of "what happens in this 'private' branch is more than an implementation 'detail' but a useful story and a useful documentation of the process". You don't need to agree that is "objectively" or "universally" a "better" way to make PRs for everyone and every project, but I'd hope you could at least respect that it's a nice goal that some of us have at least some of the time and why we would like PR tools that respect that approach as much as they seem to already respect your "no one cares how the sausage is made" approach.

Reply on news.ycombinator.com