Rustc-dev-guide: PR follow-up commits strategy at odds with comment from rust-highfive bot

[mightyiam]

https://github.com/rust-lang/rustc-dev-guide/blob/6b5c62c6f8946a1a158bb1e84e1dabe87daee820/src/contributing.md

Is this paragraph

Rust follows a no merge-commit policy, meaning, when you encounter merge conflicts you are expected to always rebase instead of merging. E.g. always use rebase when bringing the latest changes from the master branch to your feature branch. Also, please make sure that fixup commits are squashed into other related commits with meaningful commit messages.

At odds with the bot comment https://github.com/rust-lang/rust/pull/77219#issuecomment-699454142 ?

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

[jyn514]

The problem is that different reviewers have different preferences for how you handle git 😅 For example @Mark-Simulacrum prefers commits to be squashed, but I prefer extra commits so I don't have to worry about https://github.com/isaacs/github/issues/1834.

Maybe we could change it to say 'squash commits before it's merged'? I don't think there's a great universal guideline here.

[Mark-Simulacrum]

I prefer the commits to be squashed mainly because I find it much easier to re-review PRs rather than trying to remember past state, usually because I go 1-2 weeks at least between reviews on a given PR and I need to figure out what was going on. It's much easier to do so when things are in a consistent state within each commit rather than fixup commits scattered in git history.

On small PRs, it's also true that I personally will not approve things with stray commits without them being squashed in, so if the author is constantly squashing we can save a cycle at the end.

[LeSeulArtichaut]

I think squashing commits right before merging makes the most sense

[mightyiam]

Is it OK if the commit strategy depends on the assigned reviewer?

[camelid]

I think squashing commits right before merging makes the most sense

Sometimes on larger PRs it's helpful to squash once or twice in the middle. E.g. I had a PR that got up to 60 commits because we kept changing the approach, so it was helpful to squash before the final review.