250 lines
10 KiB
Plaintext
250 lines
10 KiB
Plaintext
===============
|
|
Committing code
|
|
===============
|
|
|
|
This section is addressed to the committers 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 now hosted at GitHub, most 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 however, committers - 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 test the pull request with one of the pull request
|
|
builders that doesn't run automatically, such as Oracle or Selenium. See the
|
|
`Jenkins wiki page`_ for instructions.
|
|
|
|
.. _Jenkins wiki page: https://code.djangoproject.com/wiki/Jenkins
|
|
|
|
An easy way to checkout a pull request locally is to add an alias to your
|
|
``~/.gitconfig`` (``upstream`` is assumed to be ``django/django``)::
|
|
|
|
[alias]
|
|
pr = !sh -c \"git fetch upstream pull/${1}/head:pr/${1} && git checkout pr/${1}\"
|
|
|
|
Now you can simply 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 master.
|
|
$ git checkout master
|
|
$ git pull upstream master
|
|
$ # Rebase the pull request on master.
|
|
$ git checkout pr/####
|
|
$ git rebase master
|
|
$ git checkout master
|
|
$ # Merge the work as "fast-forward" to master 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 master
|
|
$ # Push!
|
|
$ git push upstream master
|
|
$ # Delete the pull request branch.
|
|
$ git branch -d pr/xxxx
|
|
|
|
For changes on your own branches, force push to your fork after rebasing on
|
|
master but before merging and pushing to upstream. This allows the commit
|
|
hashes on master and your branch to match which automatically closes the pull
|
|
request. Since you can't push to other contributors' branches, comment on the
|
|
pull request "Merged in XXXXXXX" (replacing with the commit hash) after you
|
|
merge it. Trac checks for this message format to indicate on the ticket page
|
|
whether or not a pull request is merged.
|
|
|
|
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 committer 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.
|
|
|
|
If the patch wasn't a pull request, you should credit the contributors in
|
|
the commit message: "Thanks A for report, B for the patch and C for the
|
|
review."
|
|
|
|
* 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.
|
|
|
|
If your commit closes a ticket and is in a branch, use the branch name
|
|
first, then the "Fixed #xxxxx." For example:
|
|
"[1.4.x] Fixed #123 -- Added whizbang feature."
|
|
|
|
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 also 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 master.
|
|
|
|
There's a `script on the wiki
|
|
<https://code.djangoproject.com/wiki/CommitterTips#AutomatingBackports>`_
|
|
to automate this.
|
|
|
|
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 committer, **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 committer 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``, just delete it.
|
|
For instance, if you did: ``git push upstream feature_antigravity``,
|
|
just do a reverse push: ``git push upstream :feature_antigravity``.
|
|
|
|
.. _ticket tracker: https://code.djangoproject.com/
|