CONTRIBUTING: clarify the reason for no force pushes

[Managor]

Checklist

• The page(s) are in the correct platform directories: common, linux, osx, windows, sunos, android, etc.
• The page description(s) have links to documentation or a homepage.
• The page(s) follow the content guidelines.
• The page(s) follow the style guide.
• The PR contains at most 5 new pages.
• The PR is authored by me, or has been human-reviewed if it was created with AI or machine translation software.
• The PR title conforms to the recommended templates.
• Version of the command being documented (if known):

Reference issue: #

[SethFalco]

Yeah this makes more sense!

Just a pet peeve, but I'm actually not a big fan of the term "subcommit" myself, as this appears to be a non-standard way to refer to commits in the source branch. 🤔

I prefer "mess" which, to me, feels obvious is an informal term, while subcommit gives me the impression it's a concept of git I missed.

I get what Krishna tried to achieve there, and think it does establish the understanding intended. However, I don't think it's worth inventing vocabulary that's rarely if ever used.

I don't feel strongly enough about it to block the PR though, but I thought it was worth sharing in case it does convince anyone to reconsider.

[waldyrious]

Thanks for tackling this, @Managor! I think this is a good direction, but I would prefer it to be a little more explicit. In particular, "so the order of events is preserved" should make it clear we refer to the evolution of the PR based on the review suggestions.

It would also be worth mentioning that GitHub's PR review UI is unable to handle force-pushes, to make it clear it's not a matter of principle, but a limitation of the tooling we use (other forges like Gerrit or GitLab, for example, are able to show various versions of a merge request, so it's easy to see what changed in each iteration.)

Finally, I would like this to acknowledge the reasons why one might want to force-push — for example, to rebase a PR branch due to the GitHub UI's explicit suggestion that the branch needs to be updated:

...but also, it's perfectly legitimate that one might want to create a clean set of commits in a PR, especially for larger changes, which would benefit from not being squash-merged. We should at least acknowledge that this is a valid approach, even if we have decided as a community to not accept it.

[waldyrious]

We should at least acknowledge that this is a valid approach, even if we have decided as a community to not accept it.

In fact, I'd go further and refrain from stating we reject this approach entirely, and make it clear it would be occasionally acceptable (and I'd say even desirable) in PRs with large/complex changes.

I would like to point out the Maintainer's Guide says about this from the other perspective:

When merging PRs, use the merge strategy that produces a clean Git history: If there's a single commit in the PR, or if the multiple commits are not semantically independent changes, use the Squash and merge method. (Don't forget to clean up the body of the squashed commit message.) If instead, the PR author took the time to craft individual, informative messages for each commit, then use the Rebase and merge method, to honor that work and preserve the history of the changes. For less clear-cut cases, a simple heuristic you can follow is that if there are more "dirty" commits than "clean" commits, then prefer squash, else do a rebase.

[Managor]

Judging if a PR would benefit from not squash merging is an additional cognitive load that I would frankly prefer not to take. It would get applied so rarely that I don't see the point of worrying myself about it.

[waldyrious]

Let's say that for PRs making individual page edits or additions we can automatically assume the commits can be squashed. That would leave only PRs changing several files requiring that decision, which I take to be a minority. Would that help?

[Managor]

Not really. The page additions and edits are almost always grouped by a topic, thus they make sense to be squashed.

[waldyrious]

I fail to see where we are disagreeing. I stated precisely that page additions and edits are squashable 🤔 Can you point out more specifically what you are disagreeing with? (And, by the way, what compromise you would find acceptable?)

[Managor]

That would leave only PRs changing several files requiring that decision, which I take to be a minority.

To me this says that when a PR changes multiple files, it should not be squashed.

[waldyrious]

You are phrasing it as if it is a black-and-white rule. All I meant to suggest was that PRs modifying multiple might be worth merging without squashing — but only if the commits are indeed cleaned up and meaningfully titled, rather than stuff like "Update <file X>" or "Address comments", which IMHO should be pretty easy to spot.

I understand that verifying that is not a zero-effort task, but it's not a huge effort either. Not to mention that under this proposed guideline, PRs modifying a single file or making the same repetitive change to multiple files (which are, AFAIK, the vast majority of tldr-pages PRs) can indeed default to be squash-merged, with no commit inspection whatsoever. I feel that paying attention to the branch history in the few cases that do more complex changes is a fair ask from the maintainers, considering that the contributor would have invested their end of the effort by taking the time to tidy up the branch history.

And once more: I'm not suggesting to give contributors carte blanche to rebase and force-push willy-nilly. I still agree with the spirit of this notice. I'm just proposing that it is not phrased in absolute terms, but does provide room for exceptions where appropriate.

[SethFalco]

To me this says that when a PR changes multiple files, it should not be squashed.

You are phrasing it as if it is a black-and-white rule. All I meant to suggest was that PRs modifying multiple might be worth merging without squashing…

Please correct me if I'm wrong, but I think an example/TL;DR (no pun intended) would help here, so I'm just adding one. I follow the same approach for all PRs I'm responsible for merging, not just in tldr-pages.

If the commit history looks like this, we can respect that the user organized their changes and wrote clean commit messages by preserving their commits:

• cat: add page
• cat: remove example
• cat: dutch translation

If the commit history looks like this, we'll mask the cluttered history by squashing the commits:

• cat: add page
• typo
• code review fixes
• fix linting
• AAAAAAAAAAAAAAAAAAAAAA

[sbrl]

I'm not so sure this improves clarity.

For example, adding within the pull request suggests that preservation of event order matters only inside the PR, and not e.g. on the main branch.

In the same vein, the mess will be contained in the PR seems a little vague compared to the previous wording, and also conflicts with the above.

[Managor]

For example, adding within the pull request suggests that preservation of event order matters only inside the PR, and not e.g. on the main branch.

I fail to see the problem here. If PR's are squashed, nobody needs to think about the main branch.

[sbrl]

From that perspective, why is it that we e.g. do a traditional merge, instead of our current policy of squash+merge?

I was under the impression that it was to ensure the history on the main branch was clean and easy to understand?

[Managor]

I don't think these are in opposition. Anyways, I made a new commit that adjusts the wording a little bit ca43858