Move away from the term "patch" to refer to a contribution/pull request in the contributor documentation

[Baptiste Mispelon]

No one effectively contributes via patches anymore (according to the Trac database, only ~10 patches were submitted in the past 12 months), I think it's time we update Trac and our documentation to reflect that.

I've created a PR to change the wording on Trac itself: ​https://github.com/django/code.djangoproject.com/pull/224

What's left now is to update our contributing guides in the documentation.

[Baptiste Mispelon]

I've attached a patch for the documentation changes 🙃

(If you insist, I've also opened a PR: ​https://github.com/django/django/pull/18780)

[Baptiste Mispelon]

I also considered replacing the expression "ready for checkin" with "ready for merging" (which has the huge advantage of having the exact same number of letters), but that would require a database migration on Trac which could be a bit annoying (though not impossible).

If you think that's worth it I can look into that as well.

[Tim Graham]

There were some recent changes along these lines in 55a2e3136b13d1af95a4129001dac963c26d8415.

I'd prefer not to rename files and cause external broken links (​Cool URIs Don't Change). (E.g. you proposed to rename ....writing-code/submitting-patches.txt to .../writing-code/pull-requests.txt.)

You also proposed to remove the ".. _patch-review-checklist:" heading which I have used extensively in Trac comments as well as on the PatchReviewChecklist wiki page (but if the page is renamed the link will break anyway).

It's possible Django will use Git, GitHub, and "pull requests" forever, but it might be worth considering if these specific terms should really be embedded in URLs.

[Claude Paroz]

I also think that "pull request" is a GitHubism. Not that we should not use it, but having "patch" here and there as a more generic term doesn't shock me.

[Sarah Boyce]

Replying to Tim Graham:

I'd prefer not to rename files and cause external broken links (​Cool URIs Don't Change). (E.g. you proposed to rename ....writing-code/submitting-patches.txt to .../writing-code/pull-requests.txt.)

You also proposed to remove the ".. _patch-review-checklist:" heading which I have used extensively in Trac comments as well as on the PatchReviewChecklist wiki page (but if the page is renamed the link will break anyway).

I agree we should be careful not to break links here

I'm in favor of updating from patch, to either "pull request"/"contribution"/"solution"/"fix"

I think "Has patch" -> "Has pull request" as when someone marks this as Yes, I expect to see something reviewable in GitHub which has also triggered our CI

"Patch needs improvement" could be "Requested changes" (I have stolen that from GitHub but is perhaps less of a GitHubism)

Will review the PR in detail for more specific cases

[Baptiste Mispelon]

Thanks everyone for the feedback, I'll try to address it point by point

1) Not breaking existing URLs

This is a very good point, but I think we should address that via setting up 301 redirects in the server configuration (this is something I can do as a member of the ops team)
I'll also note that there is some precedent for breaking docs links (found via git log --diff-filter=RD --stat -- docs/): d8b6a76bc745b21c6cf2b29c220a91bcae7fd3d7, 8eae094638acf802c8047b341d126d94bc9b45a3, or 3d14cbc86781ea1051af7f0c421bee3ecf2f9842

2) Not breaking URL fragments / heading names

I'm not opposed to re-adding some directives like .. _patch-review-checklist to try and preserve deep links people might have used, but I think it should be part of a documented policy. Why is this heading on this page important that it requires preserving? Should that apply to all references throughout the documentation? If not, what's the cutoff, how do we measure it?
I don't think it's realistic to expect headings not to ever change names, and without a policy in place (ideally enforced by some kind of automated tool) those "backwards-compatibilty" references are at risk of being removed by accident.

3) Githubisms

I would argue that the term "pull request" is general enough as to not be a "githubism" (it is used on bitbucket for example). In any case, this word reflects the reality of our contributing process today and for the foreseeable future, unlike the word "patch" which one could also call an "SVNism" .

4) Patch is a generic term

I think that one might be a matter of opinion (and mine is that it's not) which would be hard to quantify objectively. My belief is that the majority of our new contributors do not know what a "patch" is, but they know what a "pull reqest" is.

I'll also point out that the Python devguide recently made changes in a similar direction: ​https://github.com/python/devguide/commit/0ad84cdd913dfd58df1fe4862eb7bf06cc8f4882

Also also, our documentation currently uses the word "patch" for several different meanings:
1) As a synonym for "pull request" essentially
2) To point to a specific commit in the context of a security release
3) A "patch" release
4) The HTTP method

Replacing one of these (1) would be an improvement in my opinion.

[Sarah Boyce]

I'm not opposed to re-adding some directives like .. _patch-review-checklist to try and preserve deep links people might have used, but I think it should be part of a documented policy. Why is this heading on this page important that it requires preserving?

For example, ​https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#patch-review-checklist has been on an automated welcome message on GitHub PRs for first time contributors for quite a while - so this link is used and shared quite extensively. I think it's normal to share a particular version of the docs in a link, but often in the internal docs, I share "dev" links so folks get the latest information on our processes - unfortunately these links are more "brittle".
Setting a redirect would be great 👍

This is not a documented policy. I'd say we change header names (though never really touched the Wiki). References we don't change very often and file renaming/removing is also quite rare.

[Claude Paroz]

unlike the word "patch" which one could also call an "SVNism"

I don't agree, "patch" is still used in the Git world (see ​https://git-scm.com/docs/git-format-patch for example). Once again, I don't oppose to this proposal, just that we don't need to purge all "patch" occurrences in our docs. It may still make sense in some contexts.

[Tim Graham]

If "patch" has lost its meaning among new contributors, or makes people think we're using SVN, I'm very surprised.
​https://en.wikipedia.org/wiki/Patch_(computing)
​https://git-scm.com/docs/SubmittingPatches

For "pull request", I was thinking about GitLab, which uses "merge request."

[Baptiste Mispelon]

I've updated the PR after some reviews, here's the current summary:

• "patch" is replaced with a mixture of "pull request", "change" or "contribution" on all the pages that talk about contributing to Django (this matches the current effective practice and is similar to what was done on the Python devguide)
• The word "patch" is kept when talking about an actual patch file
• The word "patch" is kept in the names of some files and in some anchors, so as not to break existing links

There are also two PRs connected to this ticket:

• One for Trac itself to rename the fields on the UI (the name of the field is not affected, so things like report links will continue to work): ​https://github.com/django/code.djangoproject.com/pull/224
• One for the dashboard to rename some of the metrics: ​https://github.com/django/djangoproject.com/pull/1729

While these two PRs are related, they don't technically require to be merged/deployed in any specific order. I personally suggest waiting until the docs change in this ticket are live before deploying them.

[Natalia Bidart]

Thank you, Baptiste, for initiating this discussion!

I’m on the side of avoiding the term "pull request" (except when referring to GitHub-concrete docs, specifically the ​Working with Git and GitHub docs). As Tim mentioned, Andreu and I worked to replace several instances of "patch" in the docs ​in this patch :clown:

In that revision, my priority was to avoid breaking existing anchors and to ensure that "pull request" was not used as a direct synonym for "patch," since they have distinct meanings. I’ll be adding comments on each PR, but if we are moving away from "patch," I think it’s important to use platform-neutral terms, unless we're specifically referring to a GitHub pull request.

I also believe "patch" is not exclusive to SVN. I’ve used it for a long time to describe a diff file containing code changes, even before tools like CVS came along. GitHub itself uses the term as well, you can download the patch for any PR by appending .patch to the PR URL:

​https://github.com/django/djangoproject.com/pull/18780.patch

Additionally, I don’t see "pull request" as a generic term. It seems specific to GitHub, especially since other platforms like GitLab use ​Merge Request and Launchpad uses ​Merge Proposal. For that reason, I’d prefer not to replace "patch" with "pull request." In fact, "patch" is one of the more platform-agnostic terms we have, though I agree that we should use it carefully and in its original sense (i.e., a diff of code proposing a change).

[Baptiste Mispelon]

Thanks for your beed back Natalia!

I do not agree that "Pull request" is a github-specific term (it might have started that way, but it is not anymore). It's also used by:

• Atlassian / bitbucket [1]
• Amazon / CodeCommit [2]
• Gitea [3]
• Codeberg [4]

And for what it's worth, wikipedia also says that "pull request" and "merge request" are equivalent [5].

But even with all this, my argument is that we should meet the contributors where they are, and today that's on github, using pull requests. If and when that process changes, we can amend the documentation again.

With the current state of my merge request, no URL has changed, and the only headings that were modified did not lead to any internal links being broken. Plus there's currently no documented policy I could find around backwards-compatibility of anchors.

The change I'm proposing in this merge request is not to erase "patch" altogether, nor is it to replace all of its occurences by "pull request". The word "patch" has been kept where it reflects the reality, and replaced by a variation of "pull request", "change", "contribution", "proposal", ... where it makes more sense.

[1] ​https://www.atlassian.com/git/tutorials/making-a-pull-request
[2] ​https://docs.aws.amazon.com/codecommit/latest/userguide/pull-requests.html
[3] ​https://docs.gitea.com/next/usage/pull-request
[4] ​https://docs.codeberg.org/collaborating/pull-requests-and-git-flow/
[5] ​https://en.wikipedia.org/wiki/Distributed_version_control#Pull_requests