5 ms·

This is basically solved by squashing each PR on merge and having a good PR title + description.

theonething 4y ago
This is my habit and I advocate for it on new teams. It's so simple and effective. Why make it complicated?
ajorgensen 4y ago
But why not do both? I find that nice commit messages make it easier on the review to see whats going on rather than a bunch of wip commits. Then squash all of the commits into a single commit with a nice title and message. The benefits of the squash at the end are, assuming you require tests pass before merge, you have a history of commits all with passing builds which makes bisect possible.
- mixmastamyk 4y ago
  Wouldn't the cleanup work be lost in the squash?
hardolaf 4y ago
This is exactly what we do at my job. The only time it ever causes issues with some things is when we're ramping up a new project and the PR and ticket titles don't necessarily match the work we've done. But that's more of a problem where we could make everything bite-sized, or we can deliver complete packages in terms of a feature plus all of the miscellaneous changes that we needed to make along the way to make it work. Now, is this the best way to do things? No, you should ideally make those changes separately and push each one separately. But that slows down progress.
aulin 4y ago
why would you squash multiple informative commits with their commit messages and their informative context about your thought process into a single big squashed commit? git can merge branches and keep track of it, there is no need to squash unless you split trivial changes that belong together and want to group them before sharing them
- theonething 4y ago
  > no need to squash unless you split trivial changes that belong together If a PR represents a distinct product feature/bug fix/unit of work, one could argue that its commits belong together. Squashing makes for a very clean history. True it reduces granularity, but in my opinion at least, it's a good level of granularity. The exception to this would be a really big PR. That's a reason to avoid huge PRs.

This is my habit and I advocate for it on new teams. It's so simple and effective. Why make it complicated?

But why not do both? I find that nice commit messages make it easier on the review to see whats going on rather than a bunch of wip commits. Then squash all of the commits into a single commit with a nice title and message. The benefits of the squash at the end are, assuming you require tests pass before merge, you have a history of commits all with passing builds which makes bisect possible.

mixmastamyk 4y ago
Wouldn't the cleanup work be lost in the squash?

hardolaf 4y ago

This is exactly what we do at my job. The only time it ever causes issues with some things is when we're ramping up a new project and the PR and ticket titles don't necessarily match the work we've done. But that's more of a problem where we could make everything bite-sized, or we can deliver complete packages in terms of a feature plus all of the miscellaneous changes that we needed to make along the way to make it work. Now, is this the best way to do things? No, you should ideally make those changes separately and push each one separately. But that slows down progress.

aulin 4y ago

why would you squash multiple informative commits with their commit messages and their informative context about your thought process into a single big squashed commit? git can merge branches and keep track of it, there is no need to squash unless you split trivial changes that belong together and want to group them before sharing them

theonething 4y ago
> no need to squash unless you split trivial changes that belong together If a PR represents a distinct product feature/bug fix/unit of work, one could argue that its commits belong together. Squashing makes for a very clean history. True it reduces granularity, but in my opinion at least, it's a good level of granularity. The exception to this would be a really big PR. That's a reason to avoid huge PRs.