Nice one. Thank you!

@tomchristie
This was meant to be squashed..
Now there are three commits instead of one, and one even make git-bisect harder..!

@blueyed maybe not. Whether you end up with one commit or another, you get the associated PR which will give you the context.

?

I certainly meant it to be squashed.. what does "Possible fix" or a "fixup!" (meant for git's autosquashing) have to do in a project's history?

Also there are 3 times more commits when git-bisecting (which I've used to debug this issue itself), and one of them needs to be skipped/ignored when running all tests.

@tomchristie@xordoquy@jpadilla
Any feedback on my previous comment?

I think (especially with widely-used projects) the history of the master branch should be kept clean (for reviewers, git-bisect etc).

This also includes squashing single-commit PRs, instead of merging them.

There are a lot of arguments one way or another. Sometime it's better to have several more atomic commits to have a better understanding and I like much better merging rather than rebasing because it keeps the commit uuid which helps me to know whether a branch has been merged or not and makes it easier in an heavily distributed env.

To conclude, my thoughts on that are that it's been merged so it's history now and I doubt the extra work is worth it.

Additional side note, the commit itself isn't so much important as it leads to the PR anyway.
09f62e1 has a nice master (#5138) tag which leads to the full discussion here. Much more useful than a simple commit.

@xordoquy
Yes, the history of this PR cannot be changed of course, but my call was to have maintainers agree on some consensus in this regard.

And also raising awareness that commits with a "fixup!" prefix should not get merged as-is in general.

Re09f62e1: the commit message "Possible fix" is very bad (even worse than some "fixup!"-prefixed one).
And on GitHub you get the PR reference also for squashed commits (of course).

The PR makes sense. It has a well described context through the linked issue as well as a test case.
You did a very good job with this PR.
I'll probably take some attention to the commit messages from now on but we also have to balance that with the fact that requesting too much for contribution will turn some contributors away from the project and OSS in general could do with more contributors.

@xordoquy
Thanks.
In this case it should have been just squashed using the GitHub UI, in which case there is no extra effort required from the contributor - except for their local branch not being easily identified as being merged.
If there is uncertainty then it is best from my experience to ask for squashing and force-pushing from the maintainer, if they want to do it.