mirror of
https://github.com/django/django.git
synced 2024-12-23 17:46:27 +00:00
250 lines
10 KiB
Plaintext
250 lines
10 KiB
Plaintext
===============
|
|
Committing code
|
|
===============
|
|
|
|
This section is addressed to the mergers and to anyone interested in knowing
|
|
how code gets committed into Django. If you're a community member who wants to
|
|
contribute code to Django, look at :doc:`writing-code/working-with-git` instead.
|
|
|
|
.. _handling-pull-requests:
|
|
|
|
Handling pull requests
|
|
======================
|
|
|
|
Since Django is hosted on GitHub, patches are provided in the form of pull
|
|
requests.
|
|
|
|
When committing a pull request, make sure each individual commit matches the
|
|
commit guidelines described below. Contributors are expected to provide the
|
|
best pull requests possible. In practice mergers - who will likely be more
|
|
familiar with the commit guidelines - may decide to bring a commit up to
|
|
standard themselves.
|
|
|
|
You may want to have Jenkins or GitHub actions test the pull request with one
|
|
of the pull request builders that doesn't run automatically, such as Oracle or
|
|
Selenium. See the `CI wiki page`_ for instructions.
|
|
|
|
.. _CI wiki page: https://code.djangoproject.com/wiki/CI
|
|
|
|
If you find yourself checking out pull requests locally more often, this git
|
|
alias will be helpful::
|
|
|
|
[alias]
|
|
pr = !sh -c \"git fetch upstream pull/${1}/head:pr/${1} && git checkout pr/${1}\"
|
|
|
|
Add it to your ``~/.gitconfig``, and set ``upstream`` to be ``django/django``.
|
|
Then you can run ``git pr ####`` to checkout the corresponding pull request.
|
|
|
|
At this point, you can work on the code. Use ``git rebase -i`` and ``git
|
|
commit --amend`` to make sure the commits have the expected level of quality.
|
|
Once you're ready:
|
|
|
|
.. console::
|
|
|
|
$ # Pull in the latest changes from main.
|
|
$ git checkout main
|
|
$ git pull upstream main
|
|
$ # Rebase the pull request on main.
|
|
$ git checkout pr/####
|
|
$ git rebase main
|
|
$ git checkout main
|
|
$ # Merge the work as "fast-forward" to main to avoid a merge commit.
|
|
$ # (in practice, you can omit "--ff-only" since you just rebased)
|
|
$ git merge --ff-only pr/XXXX
|
|
$ # If you're not sure if you did things correctly, check that only the
|
|
$ # changes you expect will be pushed to upstream.
|
|
$ git push --dry-run upstream main
|
|
$ # Push!
|
|
$ git push upstream main
|
|
$ # Delete the pull request branch.
|
|
$ git branch -d pr/xxxx
|
|
|
|
Force push to the branch after rebasing on main but before merging and pushing
|
|
to upstream. This allows the commit hashes on main and the branch to match
|
|
which automatically closes the pull request.
|
|
|
|
If a pull request doesn't need to be merged as multiple commits, you can use
|
|
GitHub's "Squash and merge" button on the website. Edit the commit message as
|
|
needed to conform to :ref:`the guidelines <committing-guidelines>` and remove
|
|
the pull request number that's automatically appended to the message's first
|
|
line.
|
|
|
|
When rewriting the commit history of a pull request, the goal is to make
|
|
Django's commit history as usable as possible:
|
|
|
|
* If a patch contains back-and-forth commits, then rewrite those into one.
|
|
For example, if a commit adds some code and a second commit fixes stylistic
|
|
issues introduced in the first commit, those commits should be squashed
|
|
before merging.
|
|
|
|
* Separate changes to different commits by logical grouping: if you do a
|
|
stylistic cleanup at the same time as you do other changes to a file,
|
|
separating the changes into two different commits will make reviewing
|
|
history easier.
|
|
|
|
* Beware of merges of upstream branches in the pull requests.
|
|
|
|
* Tests should pass and docs should build after each commit. Neither the
|
|
tests nor the docs should emit warnings.
|
|
|
|
* Trivial and small patches usually are best done in one commit. Medium to
|
|
large work may be split into multiple commits if it makes sense.
|
|
|
|
Practicality beats purity, so it is up to each merger to decide how much
|
|
history mangling to do for a pull request. The main points are engaging the
|
|
community, getting work done, and having a usable commit history.
|
|
|
|
.. _committing-guidelines:
|
|
|
|
Committing guidelines
|
|
=====================
|
|
|
|
In addition, please follow the following guidelines when committing code to
|
|
Django's Git repository:
|
|
|
|
* Never change the published history of ``django/django`` branches by force
|
|
pushing. If you absolutely must (for security reasons for example), first
|
|
discuss the situation with the team.
|
|
|
|
* For any medium-to-big changes, where "medium-to-big" is according to
|
|
your judgment, please bring things up on the |django-developers|
|
|
mailing list before making the change.
|
|
|
|
If you bring something up on |django-developers| and nobody responds,
|
|
please don't take that to mean your idea is great and should be
|
|
implemented immediately because nobody contested it. Everyone doesn't always
|
|
have a lot of time to read mailing list discussions immediately, so you may
|
|
have to wait a couple of days before getting a response.
|
|
|
|
* Write detailed commit messages in the past tense, not present tense.
|
|
|
|
* Good: "Fixed Unicode bug in RSS API."
|
|
* Bad: "Fixes Unicode bug in RSS API."
|
|
* Bad: "Fixing Unicode bug in RSS API."
|
|
|
|
The commit message should be in lines of 72 chars maximum. There should be
|
|
a subject line, separated by a blank line and then paragraphs of 72 char
|
|
lines. The limits are soft. For the subject line, shorter is better. In the
|
|
body of the commit message more detail is better than less:
|
|
|
|
.. code-block:: none
|
|
|
|
Fixed #18307 -- Added git workflow guidelines.
|
|
|
|
Refactored the Django's documentation to remove mentions of SVN
|
|
specific tasks. Added guidelines of how to use Git, GitHub, and
|
|
how to use pull request together with Trac instead.
|
|
|
|
Credit the contributors in the commit message: "Thanks A for the report and B
|
|
for review." Use git's `Co-Authored-By`_ as appropriate.
|
|
|
|
.. _Co-Authored-By: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors
|
|
|
|
* For commits to a branch, prefix the commit message with the branch name.
|
|
For example: "[1.4.x] Fixed #xxxxx -- Added support for mind reading."
|
|
|
|
* Limit commits to the most granular change that makes sense. This means,
|
|
use frequent small commits rather than infrequent large commits. For
|
|
example, if implementing feature X requires a small change to library Y,
|
|
first commit the change to library Y, then commit feature X in a separate
|
|
commit. This goes a *long way* in helping everyone follow your changes.
|
|
|
|
* Separate bug fixes from feature changes. Bugfixes may need to be backported
|
|
to the stable branch, according to :ref:`supported-versions-policy`.
|
|
|
|
* If your commit closes a ticket in the Django `ticket tracker`_, begin
|
|
your commit message with the text "Fixed #xxxxx", where "xxxxx" is the
|
|
number of the ticket your commit fixes. Example: "Fixed #123 -- Added
|
|
whizbang feature.". We've rigged Trac so that any commit message in that
|
|
format will automatically close the referenced ticket and post a comment
|
|
to it with the full commit message.
|
|
|
|
For the curious, we're using a `Trac plugin`_ for this.
|
|
|
|
.. note::
|
|
|
|
Note that the Trac integration doesn't know anything about pull requests.
|
|
So if you try to close a pull request with the phrase "closes #400" in your
|
|
commit message, GitHub will close the pull request, but the Trac plugin
|
|
will not close the same numbered ticket in Trac.
|
|
|
|
.. _Trac plugin: https://github.com/trac-hacks/trac-github
|
|
|
|
* If your commit references a ticket in the Django `ticket tracker`_ but
|
|
does *not* close the ticket, include the phrase "Refs #xxxxx", where "xxxxx"
|
|
is the number of the ticket your commit references. This will automatically
|
|
post a comment to the appropriate ticket.
|
|
|
|
* Write commit messages for backports using this pattern:
|
|
|
|
.. code-block:: none
|
|
|
|
[<Django version>] Fixed <ticket> -- <description>
|
|
|
|
Backport of <revision> from <branch>.
|
|
|
|
For example:
|
|
|
|
.. code-block:: none
|
|
|
|
[1.3.x] Fixed #17028 -- Changed diveintopython.org -> diveintopython.net.
|
|
|
|
Backport of 80c0cbf1c97047daed2c5b41b296bbc56fe1d7e3 from main.
|
|
|
|
There's a `script on the wiki
|
|
<https://code.djangoproject.com/wiki/MergerTips#AutomatingBackports>`_ to
|
|
automate this.
|
|
|
|
If the commit fixes a regression, include this in the commit message:
|
|
|
|
.. code-block:: none
|
|
|
|
Regression in 6ecccad711b52f9273b1acb07a57d3f806e93928.
|
|
|
|
(use the commit hash where the regression was introduced).
|
|
|
|
Reverting commits
|
|
=================
|
|
|
|
Nobody's perfect; mistakes will be committed.
|
|
|
|
But try very hard to ensure that mistakes don't happen. Just because we have a
|
|
reversion policy doesn't relax your responsibility to aim for the highest
|
|
quality possible. Really: double-check your work, or have it checked by
|
|
another merger **before** you commit it in the first place!
|
|
|
|
When a mistaken commit is discovered, please follow these guidelines:
|
|
|
|
* If possible, have the original author revert their own commit.
|
|
|
|
* Don't revert another author's changes without permission from the
|
|
original author.
|
|
|
|
* Use git revert -- this will make a reverse commit, but the original
|
|
commit will still be part of the commit history.
|
|
|
|
* If the original author can't be reached (within a reasonable amount
|
|
of time -- a day or so) and the problem is severe -- crashing bug,
|
|
major test failures, etc. -- then ask for objections on the
|
|
|django-developers| mailing list then revert if there are none.
|
|
|
|
* If the problem is small (a feature commit after feature freeze,
|
|
say), wait it out.
|
|
|
|
* If there's a disagreement between the merger and the reverter-to-be then try
|
|
to work it out on the |django-developers| mailing list. If an agreement can't
|
|
be reached then it should be put to a vote.
|
|
|
|
* If the commit introduced a confirmed, disclosed security
|
|
vulnerability then the commit may be reverted immediately without
|
|
permission from anyone.
|
|
|
|
* The release branch maintainer may back out commits to the release
|
|
branch without permission if the commit breaks the release branch.
|
|
|
|
* If you mistakenly push a topic branch to ``django/django``, delete it.
|
|
For instance, if you did: ``git push upstream feature_antigravity``,
|
|
do a reverse push: ``git push upstream :feature_antigravity``.
|
|
|
|
.. _ticket tracker: https://code.djangoproject.com/
|