Hacker News new | ask | show | jobs
by Lindby 6 hours ago
All the things in between my commits is a messy soup. Looking there is not useful to anyone. I rewrite my history with git rebase so each commit is small and atomic. The story I create with my commits is what explains why things are as they are, it doesn't matter if it's the true chronological story on how it actually happened.

I agree with the author that reviewing pull requests is too late. The problem with pull request is that they make it hard to review individual commits since they are geared towards reviewing the result of an entire branch at once. But the answer is not to share all the noise, it should be to encourage small atomic commits so you can review the early work before the entire feature/fix is complete.

2 comments

The way you use commits sounds like how I tend to use stacked PRs at work. Good commit hygiene is hard to enforce at a team level, but for whatever reason at the PR level people are happy to write good descriptions and keep the sets of changes tidy.
> The problem with pull request is that they make it hard to review individual commits since they are geared towards reviewing the result of an entire branch at once. But the answer is not to share all the noise, it should be to encourage small atomic commits so you can review the early work before the entire feature/fix is complete.

Isn't that just a GitHub problem, as opposed to Phabricator, Gerrit, etc.?

It is, but I think it's hard to explain to folks who have only used the Git/GitHub model – I know, I've been that person not really getting it.

I think what's missed is two things:

- In the Phabricator/Gerrit model you typically end up with changes that are smaller than a PR, but bigger than a commit.

- You lose some history, or track it in a different way. With a PR you might add code to address comments in a new commit on the end, but with Phabricator/Gerrit you don't. If you already aggressively rebase in Git to absorb changes into commits that they make sense to go in, this won't be much of a change, and some systems give you views on the history happening within each change. But if you expect to see everything like that in the Git history, you may not get that, but the workflow changes around it and that's ok.

I think both types of review are in a local maxima, where you lose some things to move to the other type, and it was fear of losing those from my workflow that made me resist the change a bit. When you get there though, you realise it's just not a problem.