Hacker News new | ask | show | jobs
by bonzini 851 days ago
> If you have a PR with 5 commits, 4 of which break the build and the last one fixes it, then merging that will be a problem if you need to git bisect later.

And the answer is that you don't; each commit is individually testable and reviewable. Changes requested by reviewers are squashed into the commits and then merged into the project. Unfortunately, while the git command line has "range-diff" to ease review with this workflow, neither GitHub not Gitlab have an equivalent in their UI.
2 comments
palata 851 days ago
Well, I was obviously meaning that "workflows that squash the commits in a PR are workflows where each individual commit is not tested/reviewed separately".

Of course, if your workflow is different, then... well it is different. Doesn't make the "squash workflows" irrational.

Disclaimer: I don't squash PRs.

link
syndicatedjelly 851 days ago
> And the answer is that you don't; each commit is individually testable and reviewable.

How does this work in practice? Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?

> Changes requested by reviewers are squashed into the commits and then merged into the project.

So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself? Who is the author of the commit at that point?

I'm trying to understand what you're talking about, because you seem to have something figured out, for a problem that every team I've worked on struggles with.

link
bonzini 851 days ago
> Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?

1) yes 2) when a PR is submitted 3) it can be a lot for a huge project-wide refactoring, but generally I would say 1 to 5 is typical and up to 20 is not strange.

> So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself?

No, the author applies the requested change and force-pushes once he has gotten all the requested changes applied.

> because you seem to have something figured out

Thanks! But it's not me—it's how Linux has used git from the beginning, for example. In fact it's the only workflow that is used by projects that still use email instead of GitHub/Gitlab PRs, but (trading some old pain with new pain) it is possible to use it even with the latter. The harder part is marching the review comments to the new patch, which is actually pretty easy to do with emails.

It's quite some work and there's some learning curve. But depending on the project it can be invaluable when debugging. It depends a lot on how much the code can be covered by tests, in particular.

link
palata 850 days ago
Yeah, I think that the email workflow (which I love) is more adapted to this!
link