> What is wrong with simply pushing a "Fix linting issues" in a new commit? It's self-contained and very well describes the (single) purpose of the commit.
Because I care a lot more about the logical history of the source code versus the actual history of the source code. Commits like "fix linting issues" are more like artifacts of the development process. The commit exists likely because you forgot to run the linter earlier, and only found the issue after creating the commit. Logically speaking, those fixes belong with the relevant change.
Now, if you're just going to squash the PR you're working on, then sure. The extra commit totally doesn't matter. Make as many of those commits as you want.
But if your PR is 5 commits that you want to "rebase & merge" as-is (which is something I do pretty frequently), and you care about the history being easy to navigate, then it can totally make sense to do fixups before merging instead of creating new "fix linting issues" commits. If the follow the latter, then your commit history winds up being littered with those sorts of things. And fewer commits will likely pass tests (because you created new commits to fix tests).
I want to be VERY CLEAR, that I do not agree with your use of the word "wrong." I do not want to call your workflow wrong. Communicating the nuances and subtleties of workflows over text here is very difficult. For example, one take-away from my comment here is that you might think I always work this way. But I don't! I use "squash & merge" plenty myself, in which case, git-absorb is less useful. And sometimes breaking a change down into small atomic commits is actually a ton of work, and in that case, I might judge it to not be worth it. And still yet other times, I might stack PRs (although that's rare because I find it very annoying). It always depends.
> But otherwise I think it's a little bit problematic since it makes it harder for the reviewer to inspect and understand what changes have been done. But it also makes it harder for a developer since not all fixes are context-free, e.g. a fix for an issue found in the PR cannot always be attributed to the self-contained and single commit in your branch but it's the composition of multiple commits that actually makes this bug appear.
I'm having a hard time understanding your concern about the reviewer. But one aspect of breaking things down into commits is absolutely to make it easier for the reviewer. Most of my PRs should be read as a sequence of changes, and not as one single diff. This is extremely useful when, in order to make a change, you first need to do some refactoring. When possible (but not always, because sometimes it's hard), I like to split the refactor into one commit and then the actual interesting change into another commit. I believe this makes it easier for reviewers, because now you don't need to mentally separate "okay this change is just refactoring, so no behavior changes" and "okay this part is where the behavioral change is."
In the case where you can't easily attribute a fix to one commit, then absolutely, create a new commit! There aren't any hard rules here. It doesn't have to be perfect. I just personally don't want to live in a world where there are tons of "fix lint" commits scattered through the project's history. But of course, this is in tension with workflow. Because there are multiple ways to avoid "fix lint" commits. One of those is "squash & merge." But if you don't want to use "squash & merge" in a particular case, then the only way to avoid "fix lint" commits is to do fixups. And git-absorb will help you find the commits to create fixups for automatically. That's it.
Because I care a lot more about the logical history of the source code versus the actual history of the source code. Commits like "fix linting issues" are more like artifacts of the development process. The commit exists likely because you forgot to run the linter earlier, and only found the issue after creating the commit. Logically speaking, those fixes belong with the relevant change.
Now, if you're just going to squash the PR you're working on, then sure. The extra commit totally doesn't matter. Make as many of those commits as you want.
But if your PR is 5 commits that you want to "rebase & merge" as-is (which is something I do pretty frequently), and you care about the history being easy to navigate, then it can totally make sense to do fixups before merging instead of creating new "fix linting issues" commits. If the follow the latter, then your commit history winds up being littered with those sorts of things. And fewer commits will likely pass tests (because you created new commits to fix tests).
I want to be VERY CLEAR, that I do not agree with your use of the word "wrong." I do not want to call your workflow wrong. Communicating the nuances and subtleties of workflows over text here is very difficult. For example, one take-away from my comment here is that you might think I always work this way. But I don't! I use "squash & merge" plenty myself, in which case, git-absorb is less useful. And sometimes breaking a change down into small atomic commits is actually a ton of work, and in that case, I might judge it to not be worth it. And still yet other times, I might stack PRs (although that's rare because I find it very annoying). It always depends.
> But otherwise I think it's a little bit problematic since it makes it harder for the reviewer to inspect and understand what changes have been done. But it also makes it harder for a developer since not all fixes are context-free, e.g. a fix for an issue found in the PR cannot always be attributed to the self-contained and single commit in your branch but it's the composition of multiple commits that actually makes this bug appear.
I'm having a hard time understanding your concern about the reviewer. But one aspect of breaking things down into commits is absolutely to make it easier for the reviewer. Most of my PRs should be read as a sequence of changes, and not as one single diff. This is extremely useful when, in order to make a change, you first need to do some refactoring. When possible (but not always, because sometimes it's hard), I like to split the refactor into one commit and then the actual interesting change into another commit. I believe this makes it easier for reviewers, because now you don't need to mentally separate "okay this change is just refactoring, so no behavior changes" and "okay this part is where the behavioral change is."
In the case where you can't easily attribute a fix to one commit, then absolutely, create a new commit! There aren't any hard rules here. It doesn't have to be perfect. I just personally don't want to live in a world where there are tons of "fix lint" commits scattered through the project's history. But of course, this is in tension with workflow. Because there are multiple ways to avoid "fix lint" commits. One of those is "squash & merge." But if you don't want to use "squash & merge" in a particular case, then the only way to avoid "fix lint" commits is to do fixups. And git-absorb will help you find the commits to create fixups for automatically. That's it.