On Wed, Aug 25, 2021 at 8:35 PM Matthew Brett <matthew.brett@gmail.com> wrote:
Hi,
On Wed, Aug 25, 2021 at 5:06 PM Stefan van der Walt <stefanv@berkeley.edu> wrote:
On Wed, Aug 25, 2021, at 07:04, Eric Larson wrote:
Some contributors are very good about structuring/restructuring their
PRs this way, but it seems to be the (rare) exception and not the rule.
This is in line with my experience. If you have developers who are
willing to craft their patches very carefully, a merge strategy makes sense. Otherwise, since PRs are meant to be focused in scope already, squashing works well in practice.
Github also implicitly encourages an append-to-PR workflow. They have
"view only what has changed" when you only append patches, and if I'm not mistaken cannot always track comments across rebase.
Perhaps most importantly, I think it is unreasonable to expect newcomers
to rebase their work when merge is so much easier. So, for those PRs we should squash anyway.
Eric, I agree with what you wrote. For `git bisect` the
`--first-parent` flag could be useful in only evaluating commits on the root of the commit tree.
Surely the `--first-parent` option to git bisect switches the argument back in favor of merge rather than squash, in that you can get the advantages of squash from that flag, without the disadvantage of having to go back to the pull request history on the Github interface, or by recovering the PR history somehow.
It's surely true that many less experienced developers don't give a clean history, but for those that do, it's a clear advantage to have it available with a standard checkout.
So it still seems to me that a simple 'merge or squash, it's your call' policy is the best one.
I agree with this, the maintainer should apply some judgement. There are cases where it's a toss-up (like single-commit PRs), there are messy PRs that clearly need squashing, and there are PRs with good granular commits that should be preserved. Cheers, Ralf