diff options
Diffstat (limited to 'docs/internals/contributing/committing-code.txt')
| -rw-r--r-- | docs/internals/contributing/committing-code.txt | 146 |
1 files changed, 123 insertions, 23 deletions
diff --git a/docs/internals/contributing/committing-code.txt b/docs/internals/contributing/committing-code.txt index 241a4b2418..7912ff6e9c 100644 --- a/docs/internals/contributing/committing-code.txt +++ b/docs/internals/contributing/committing-code.txt @@ -32,11 +32,91 @@ Decisions on new committers will follow the process explained in existing committer privately. Public requests for commit access are potential flame-war starters, and will be ignored. +Handling pull requests +---------------------- + +Since Django is now hosted at GitHub, many 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. However, in practice, +committers are more familiar with the commit guidelines, and they may have to +rewrite the commit history. + +Here is one way to commit a pull request:: + + # Create a new branch tracking upstream/master -- upstream is assumed + # to be django/django. + git checkout -b pull_xxxx upstream/master + + # Download the patches from github and apply them. + curl https://github.com/django/django/pull/XXXX.patch | git am + +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:: + + # Make sure master is ready to receive changes. + git checkout master + git pull upstream master + # Merge the work as "fast-forward" to master, to avoid a merge commit. + git merge --ff-only pull_xx + # Check that only the changes you expect will be pushed to upstream. + git push --dry-run upstream master + # Push! + git push upstream master + + # Get rid of the pull_xxxx branch. + git branch -d pull_xxxx + +An alternative is to add the contributor's repository as a new remote, do a +checkout of the branch and work from there:: + + git remote add <contributor> https://github.com/<contributor>/django.git + git checkout pull_xxxx <contributor> <contributor's pull request branch> + +At this point, you can work on the code and continue as above. + +GitHub provides a one-click merge functionality for pull requests. This should +only be used if the pull request is 100% ready, and you have checked it for +errors (or trust the request maker enough to skip checks). Currently, it isn't +possible to control that the tests pass and that the docs build without +downloading the changes to your developement environment. + +When rewriting the commit history of a pull request, the goal is to make +Django's commit history is as usable as possible: + +* If a patch contains back-and-forth commits, then rewrite those into one. + Typically, a commit can add some code, and a second commit can fix + stylistic issues introduced in the first commit. + +* Separate changes to different commits by logical grouping: if you do a + stylistic cleanup at the same time you do other changes to a file, + separating the changes to 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 should be split in multiple commits if possible. + +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 an usable commit history. + +.. _committing-guidlines: + Committing guidelines --------------------- -Please follow these guidelines when committing code to Django's Subversion -repository: +In addition, please follow the following guidelines when committing code to +Django's Git repository: + +* Never change the published history of django/django branches! **Never force- + push your changes to django/django.** If you absolutely must (for security + reasons for example) first discuss the situation with the core team. * For any medium-to-big changes, where "medium-to-big" is according to your judgment, please bring things up on the `django-developers`_ @@ -55,8 +135,23 @@ repository: * 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:: + + 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: "magic-removal: Added support for mind reading." + For example: "[1.4.x] Fixed #NNNNN -- 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 @@ -65,31 +160,29 @@ repository: separate commit. This goes a *long way* in helping all core Django developers follow your changes. -* Separate bug fixes from feature changes. - - Bug fixes need to be added to the current bugfix branch as well as the - current trunk. +* Separate bug fixes from feature changes. Bugfixes may need to be backported + to the stable branch, according to the :ref:`backwards-compatibility policy + <backwards-compatibility-policy>`. * If your commit closes a ticket in the Django `ticket tracker`_, begin - your commit message with the text "Fixed #abc", where "abc" is the + your commit message with the text "Fixed #NNNNN", where "NNNNN" is the number of the ticket your commit fixes. Example: "Fixed #123 -- Added - support for foo". We've rigged Subversion and 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. + 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 #abc." For example: - "magic-removal: Fixed #123 -- Added whizbang feature." + first, then the "Fixed #NNNNN." For example: + "[1.4.x] Fixed #123 -- Added whizbang feature." - For the curious: we're using a `Trac post-commit hook`_ for this. + For the curious, we're using a `Trac plugin`_ for this. - .. _Trac post-commit hook: http://trac.edgewall.org/browser/trunk/contrib/trac-svn-post-commit-hook.cmd + .. _Trac plugin: https://github.com/aaugustin/trac-github * If your commit references a ticket in the Django `ticket tracker`_ but - does *not* close the ticket, include the phrase "Refs #abc", where "abc" - is the number of the ticket your commit references. We've rigged - Subversion and Trac so that any commit message in that format will - automatically post a comment to the appropriate ticket. + does *not* close the ticket, include the phrase "Refs #NNNNN", where "NNNNN" + 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:: @@ -99,9 +192,9 @@ repository: For example:: - [1.3.X] Fixed #17028 - Changed diveintopython.org -> diveintopython.net. + [1.3.x] Fixed #17028 - Changed diveintopython.org -> diveintopython.net. - Backport of r17115 from trunk. + Backport of 80c0cbf1c97047daed2c5b41b296bbc56fe1d7e3 from trunk. Reverting commits ----------------- @@ -111,14 +204,17 @@ discovered, please follow these guidelines: * 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 before - you commit it in the first place! + the highest quality possible. Really: double-check your work, or have + it checked by another committer, before you commit it in the first place! * If possible, have the original author revert his/her 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 @@ -139,5 +235,9 @@ discovered, please follow these guidelines: * 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``. + .. _django-developers: http://groups.google.com/group/django-developers .. _ticket tracker: https://code.djangoproject.com/newticket |