I'm actually not arguing the work-in-progess commits should be kept. On the contrary, I do agree that work-in-progress commits are useless, provide no value and should never be submitted for review in a pull request.
I think I should have been more explicit when saying "your commit history is as important as your code". Because I definitely agree that your commit history, representing all the thought process you went through while developing whatever you're coding, does not have its place in the history, and I, as a colleague and reviewer of your code, couldn't care less about it.
I make a distinction between "WIP commits" and "atomic" commits that have well defined boundaries.
While developers work on their feature branches, they should be free of committing however they want, making "WIPs" commits left and right, adding code in one commit, removing that same code in a subsequent one, refactor, type terrible commit messages with no meaning, or not committing at all even.
However, once a feature is ready to be reviewed (and then released), I'm arguing that developers, before submitting their work, should re-organize their commit history: remove WIPs commits, remove refactors that happened during development, squash distinct commits that are part of the same logic, amend commit messages, etc.
The final commit history shouldn't be a series of commits that represent everything that went through the mind of the dev who coded the feature: that's not what I'm arguing for.
The final commit history should simply be multiple small digestible and reviewable chunks of work, that make sense on their own and in the context of the pull request they are part of. Those commits should represent the code in its final, best, most well thought out form.
And once you've gone through the trouble of organizing your work into meaningful, atomic, digestible, chunks of work, with appropriate descriptive commit messages, there's no reason not to merge that work.
As it was useful to the reviewer of your code during the pull-request review, it will also be useful many months later, when you need to review that same old code that seems to be having a bug or to understand why it was written the way it is.
> However, once a feature is ready to be reviewed (and then released), I'm arguing that developers, before submitting their work, should re-organize their commit history: remove WIPs commits, remove refactors that happened during development, squash distinct commits that are part of the same logic, amend commit messages, etc.
I agree that this is the optimal solution but isn't this a hefty piece of work that even might introduce subtle bugs?
Eg. I'm thinking of the overall feature needs work to be done in two separate "areas" of the code base and your wip-commits are touching these two areas "separately". Then when you are finished, reordering and squashing the wip-commits into two atomic commits seem like an obvious thing to do. But in a minority of commits you change that tiny little shared part back and forth, from both contexts. Finally you figure out how the shared part should work in the final wip. When you try to reorder the commits the changes in the shared part changes order with leads to merge conflicts when rebasing/reordering/squashing that needs to be solved.
So I'm on the fence of doing that extra work which in practicality could involve an unknown amount of "made up work" to get the clean atomic commits. And those commits then risk being a lie since the reordering, like the shared part in my contrived example, could have made that the first "atomic commit" that never existed also has a bug that never existed that someone down the line potentially stumbles upon when bisecting.
> Finally you figure out how the shared part should work in the final wip. When you try to reorder the commits the changes in the shared part changes order with leads to merge conflicts when rebasing/reordering/squashing that needs to be solved.
An alternative approach is to create a new branch off of master (or whatever the base branch is) and take the diff of the HEAD of your branch with your work in progress commits and stage the hunks of the diff to make a commit that contains a logical change (and repeat the process for the remaining parts of the diff). The git apply command us very useful in this regard.
I think I should have been more explicit when saying "your commit history is as important as your code". Because I definitely agree that your commit history, representing all the thought process you went through while developing whatever you're coding, does not have its place in the history, and I, as a colleague and reviewer of your code, couldn't care less about it.
I make a distinction between "WIP commits" and "atomic" commits that have well defined boundaries.
While developers work on their feature branches, they should be free of committing however they want, making "WIPs" commits left and right, adding code in one commit, removing that same code in a subsequent one, refactor, type terrible commit messages with no meaning, or not committing at all even.
However, once a feature is ready to be reviewed (and then released), I'm arguing that developers, before submitting their work, should re-organize their commit history: remove WIPs commits, remove refactors that happened during development, squash distinct commits that are part of the same logic, amend commit messages, etc.
The final commit history shouldn't be a series of commits that represent everything that went through the mind of the dev who coded the feature: that's not what I'm arguing for.
The final commit history should simply be multiple small digestible and reviewable chunks of work, that make sense on their own and in the context of the pull request they are part of. Those commits should represent the code in its final, best, most well thought out form.
And once you've gone through the trouble of organizing your work into meaningful, atomic, digestible, chunks of work, with appropriate descriptive commit messages, there's no reason not to merge that work.
As it was useful to the reviewer of your code during the pull-request review, it will also be useful many months later, when you need to review that same old code that seems to be having a bug or to understand why it was written the way it is.
Well, at least, IMO.