Permalink
Browse files

Fixed #18436 -- Updated contributing docs for git.

Most of the credit for this large patch goes to Anssi Kääriäinen.
Many thanks to all the people who contributed to the discussion.
  • Loading branch information...
1 parent 1a412dd commit 90fb6a46485d4f3c70d3864ab0a0e2f619449d31 @aaugustin aaugustin committed Jun 7, 2012
View
@@ -204,7 +204,7 @@ The Django open-source project
:doc:`How to get involved <internals/contributing/index>` |
:doc:`The release process <internals/release-process>` |
:doc:`Team of committers <internals/committers>` |
- :doc:`The Django source code repository <internals/svn>`
+ :doc:`The Django source code repository <internals/git>`
* **Design philosophies:**
:doc:`Overview <misc/design-philosophies>`
@@ -149,9 +149,8 @@ description.
As with most open-source projects, code talks. If you are willing to write the
code for the feature yourself or, even better, if you've already written it,
-it's much more likely to be accepted. If it's a large feature that might need
-multiple developers, we're always happy to give you an experimental branch in
-our repository; see the :doc:`writing-code/branch-policy`.
+it's much more likely to be accepted. Just fork Django on GitHub, create a
+feature branch, and show us your work!
See also: :ref:`documenting-new-features`.
@@ -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
@reinout

reinout Jun 7, 2012

What about "Handling pull requests (for core developers)"? It took me a couple of sentences before I understood that this wasn't about how I (as a non-core dev) could prepare a pull request.

@aaugustin

aaugustin Jun 8, 2012

Owner

I've clarified this in d2ad3b0

+----------------------
+
+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>
@Fruneau

Fruneau Jun 7, 2012

You can also fetch a specific branch from a repository without having to create a new remote. That's useful when there are several patches from a one-time contributor:

git fetch https://github.com//django.git <contributor's pull request branch> && git checkout -b pull_xxxx FETCH_HEAD

+
+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
@@ -1,171 +0,0 @@
-=============
-Branch policy
-=============
-
-In general, the trunk must be kept stable. People should be able to run
-production sites against the trunk at any time. Additionally, commits to trunk
-ought to be as atomic as possible -- smaller changes are better. Thus, large
-feature changes -- that is, changes too large to be encapsulated in a single
-patch, or changes that need multiple eyes on them -- must happen on dedicated
-branches.
-
-This means that if you want to work on a large feature -- anything that would
-take more than a single patch, or requires large-scale refactoring -- you need
-to do it on a feature branch. Our development process recognizes two options
-for feature branches:
-
-1. Feature branches using a distributed revision control system like
- Git_, Mercurial_, Bazaar_, etc.
-
- If you're familiar with one of these tools, this is probably your best
- option since it doesn't require any support or buy-in from the Django
- core developers.
-
- However, do keep in mind that Django will continue to use Subversion
- for the foreseeable future, and this will naturally limit the
- recognition of your branch. Further, if your branch becomes eligible
- for merging to trunk you'll need to find a core developer familiar
- with your DVCS of choice who'll actually perform the merge.
-
- If you do decided to start a distributed branch of Django and choose to
- make it public, please add the branch to the `Django branches`_ wiki
- page.
-
-2. Feature branches using SVN have a higher bar. If you want a branch
- in SVN itself, you'll need a "mentor" among the :doc:`core committers
- </internals/committers>`. This person is responsible for actually
- creating the branch, monitoring your process (see below), and
- ultimately merging the branch into trunk.
-
- If you want a feature branch in SVN, you'll need to ask in
- `django-developers`_ for a mentor.
-
-.. _git: http://git-scm.com/
-.. _mercurial: http://mercurial.selenic.com/
-.. _bazaar: http://bazaar.canonical.com/
-.. _django branches: https://code.djangoproject.com/wiki/DjangoBranches
-
-Branch rules
-------------
-
-We've got a few rules for branches born out of experience with what makes a
-successful Django branch.
-
-DVCS branches are obviously not under central control, so we have no way of
-enforcing these rules. However, if you're using a DVCS, following these rules
-will give you the best chance of having a successful branch (read: merged back
-to trunk).
-
-Developers with branches in SVN, however, **must** follow these rules. The
-branch mentor will keep on eye on the branch and **will delete it** if these
-rules are broken.
-
-* Only branch entire copies of the Django tree, even if work is only
- happening on part of that tree. This makes it painless to switch to a
- branch.
-
-* Merge changes from trunk no less than once a week, and preferably every
- couple-three days.
-
- In our experience, doing regular trunk merges is often the difference
- between a successful branch and one that fizzles and dies.
-
- If you're working on an SVN branch, you should be using `svnmerge.py`_
- to track merges from trunk.
-
-* Keep tests passing and documentation up-to-date. As with patches,
- we'll only merge a branch that comes with tests and documentation.
-
-.. _svnmerge.py: http://www.orcaware.com/svn/wiki/Svnmerge.py
-
-Once the branch is stable and ready to be merged into the trunk, alert
-`django-developers`_.
-
-After a branch has been merged, it should be considered "dead"; write access
-to it will be disabled, and old branches will be periodically "trimmed."
-To keep our SVN wrangling to a minimum, we won't be merging from a given
-branch into the trunk more than once.
-
-Using branches
---------------
-
-To use a branch, you'll need to do two things:
-
-* Get the branch's code through Subversion.
-
-* Point your Python ``site-packages`` directory at the branch's version of
- the ``django`` package rather than the version you already have
- installed.
-
-Getting the code from Subversion
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-To get the latest version of a branch's code, check it out using Subversion:
-
-.. code-block:: bash
-
- svn co https://code.djangoproject.com/svn/django/branches/<branch>/
-
-...where ``<branch>`` is the branch's name. See the `list of branch names`_.
-
-Alternatively, you can automatically convert an existing directory of the
-Django source code as long as you've checked it out via Subversion. To do the
-conversion, execute this command from within your ``django`` directory:
-
-.. code-block:: bash
-
- svn switch https://code.djangoproject.com/svn/django/branches/<branch>/
-
-The advantage of using ``svn switch`` instead of ``svn co`` is that the
-``switch`` command retains any changes you might have made to your local copy
-of the code. It attempts to merge those changes into the "switched" code. The
-disadvantage is that it may cause conflicts with your local changes if the
-"switched" code has altered the same lines of code.
-
-(Note that if you use ``svn switch``, you don't need to point Python at the
-new version, as explained in the next section.)
-
-.. _list of branch names: https://code.djangoproject.com/browser/django/branches
-
-.. _pointing-python-at-the-new-django-version:
-
-Pointing Python at the new Django version
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Once you've retrieved the branch's code, you'll need to change your Python
-``site-packages`` directory so that it points to the branch version of the
-``django`` directory. (The ``site-packages`` directory is somewhere such as
-``/usr/lib/python2.7/site-packages`` or
-``/usr/local/lib/python2.7/site-packages`` or ``C:\Python\site-packages``.)
-
-The simplest way to do this is by renaming the old ``django`` directory to
-``django.OLD`` and moving the trunk version of the code into the directory
-and calling it ``django``.
-
-Alternatively, you can use a symlink called ``django`` that points to the
-location of the branch's ``django`` package. If you want to switch back, just
-change the symlink to point to the old code.
-
-A third option is to use a path file (``<something>.pth``). This is a feature of
-the :mod:`site` module. First, make sure there are no files, directories or
-symlinks named ``django`` in your ``site-packages`` directory. Then create a
-text file named ``django.pth`` and save it to your ``site-packages`` directory.
-That file should contain a path to your copy of Django on a single line and
-optional comments. Here is an example that points to multiple branches. Just
-uncomment the line for the branch you want to use ('trunk' in this example) and
-make sure all other lines are commented::
-
- # Trunk is a svn checkout of:
- # https://code.djangoproject.com/svn/django/trunk/
- #
- /path/to/trunk
-
- # <branch> is a svn checkout of:
- # https://code.djangoproject.com/svn/django/branches/<branch>/
- #
- #/path/to/<branch>
-
- # On windows a path may look like this:
- # C:/path/to/<branch>
-
-.. _django-developers: http://groups.google.com/group/django-developers
Oops, something went wrong.

1 comment on commit 90fb6a4

Member

ptone commented on 90fb6a4 Jun 7, 2012

This actually fixes #18307 and #18436 does it not?

Please sign in to comment.