267 lines
11 KiB
Plaintext
267 lines
11 KiB
Plaintext
==================
|
|
Submitting patches
|
|
==================
|
|
|
|
We're always grateful for patches to Django's code. Indeed, bug reports
|
|
with associated patches will get fixed *far* more quickly than those
|
|
without patches.
|
|
|
|
Typo fixes and trivial documentation changes
|
|
--------------------------------------------
|
|
|
|
If you are fixing a really trivial issue, for example changing a word in the
|
|
documentation, the preferred way to provide the patch is using GitHub pull
|
|
requests without a Trac ticket. Trac tickets are still acceptable.
|
|
|
|
See the :doc:`working-with-git` for more details on how to use pull requests.
|
|
|
|
"Claiming" tickets
|
|
------------------
|
|
|
|
In an open-source project with hundreds of contributors around the world, it's
|
|
important to manage communication efficiently so that work doesn't get
|
|
duplicated and contributors can be as effective as possible.
|
|
|
|
Hence, our policy is for contributors to "claim" tickets in order to let other
|
|
developers know that a particular bug or feature is being worked on.
|
|
|
|
If you have identified a contribution you want to make and you're capable of
|
|
fixing it (as measured by your coding ability, knowledge of Django internals
|
|
and time availability), claim it by following these steps:
|
|
|
|
* `Create an account`_ to use in our ticket system. If you have an account
|
|
but have forgotten your password, you can reset it using the
|
|
`password reset page`_.
|
|
|
|
* If a ticket for this issue doesn't exist yet, create one in our
|
|
`ticket tracker`_.
|
|
|
|
* If a ticket for this issue already exists, make sure nobody else has
|
|
claimed it. To do this, look at the "Owned by" section of the ticket.
|
|
If it's assigned to "nobody," then it's available to be claimed.
|
|
Otherwise, somebody else is working on this ticket, and you either find
|
|
another bug/feature to work on, or contact the developer working on the
|
|
ticket to offer your help.
|
|
|
|
* Log into your account, if you haven't already, by clicking "Login" in
|
|
the upper right of the ticket page.
|
|
|
|
* Claim the ticket:
|
|
|
|
1. click the "assign to myself" radio button under "Action" near the bottom of the
|
|
page,
|
|
2. then click "Submit changes."
|
|
|
|
.. note::
|
|
The Django software foundation requests that anyone contributing more than
|
|
a trivial patch to Django sign and submit a `Contributor License
|
|
Agreement`_, this ensures that the Django Software Foundation has clear
|
|
license to all contributions allowing for a clear license for all users.
|
|
|
|
.. _Create an account: https://www.djangoproject.com/accounts/register/
|
|
.. _password reset page: https://www.djangoproject.com/accounts/password/reset/
|
|
.. _Contributor License Agreement: https://www.djangoproject.com/foundation/cla/
|
|
|
|
Ticket claimers' responsibility
|
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
|
|
Once you've claimed a ticket, you have a responsibility to work on that ticket
|
|
in a reasonably timely fashion. If you don't have time to work on it, either
|
|
unclaim it or don't claim it in the first place!
|
|
|
|
If there's no sign of progress on a particular claimed ticket for a week or
|
|
two, another developer may ask you to relinquish the ticket claim so that it's
|
|
no longer monopolized and somebody else can claim it.
|
|
|
|
If you've claimed a ticket and it's taking a long time (days or weeks) to code,
|
|
keep everybody updated by posting comments on the ticket. If you don't provide
|
|
regular updates, and you don't respond to a request for a progress report,
|
|
your claim on the ticket may be revoked.
|
|
|
|
As always, more communication is better than less communication!
|
|
|
|
Which tickets should be claimed?
|
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
|
|
Of course, going through the steps of claiming tickets is overkill in some
|
|
cases.
|
|
|
|
In the case of small changes, such as typos in the documentation or
|
|
small bugs that will only take a few minutes to fix, you don't need to jump
|
|
through the hoops of claiming tickets. Just submit your patch and be done with
|
|
it.
|
|
|
|
Of course, it is *always* acceptable, regardless whether someone has claimed it
|
|
or not, to submit patches to a ticket if you happen to have a patch ready.
|
|
|
|
.. _patch-style:
|
|
|
|
Patch style
|
|
-----------
|
|
|
|
Make sure that any contribution you do fulfills at least the following
|
|
requirements:
|
|
|
|
* The code required to fix a problem or add a feature is an essential part
|
|
of a patch, but it is not the only part. A good patch should also include a
|
|
:doc:`regression test <unit-tests>` to validate the behavior that has been
|
|
fixed and to prevent the problem from arising again. Also, if some tickets
|
|
are relevant to the code that you've written, mention the ticket numbers in
|
|
some comments in the test so that one can easily trace back the relevant
|
|
discussions after your patch gets committed, and the tickets get closed.
|
|
|
|
* If the code associated with a patch adds a new feature, or modifies
|
|
behavior of an existing feature, the patch should also contain
|
|
documentation.
|
|
|
|
You can use either GitHub branches and pull requests or direct patches
|
|
to publish your work. If you use the Git workflow, then you should
|
|
announce your branch in the ticket by including a link to your branch.
|
|
When you think your work is ready to be merged in create a pull request.
|
|
|
|
See the :doc:`working-with-git` documentation for mode details.
|
|
|
|
You can also use patches in Trac. When using this style, follow these
|
|
guidelines.
|
|
|
|
* Submit patches in the format returned by the ``git diff`` command.
|
|
An exception is for code changes that are described more clearly in
|
|
plain English than in code. Indentation is the most common example; it's
|
|
hard to read patches when the only difference in code is that it's
|
|
indented.
|
|
|
|
* Attach patches to a ticket in the `ticket tracker`_, using the "attach
|
|
file" button. Please *don't* put the patch in the ticket description
|
|
or comment unless it's a single line patch.
|
|
|
|
* Name the patch file with a ``.diff`` extension; this will let the ticket
|
|
tracker apply correct syntax highlighting, which is quite helpful.
|
|
|
|
Regardless of the way you submit your work, follow these steps.
|
|
|
|
* Make sure your code matches our :doc:`coding-style`.
|
|
|
|
* Check the "Has patch" box on the ticket details. This will make it
|
|
obvious that the ticket includes a patch, and it will add the ticket to
|
|
the `list of tickets with patches`_.
|
|
|
|
|
|
Non-trivial patches
|
|
-------------------
|
|
|
|
A "non-trivial" patch is one that is more than a simple bug fix. It's a patch
|
|
that introduces Django functionality and makes some sort of design decision.
|
|
|
|
If you provide a non-trivial patch, include evidence that alternatives have
|
|
been discussed on |django-developers|.
|
|
|
|
If you're not sure whether your patch should be considered non-trivial, just
|
|
ask.
|
|
|
|
Deprecating a feature
|
|
---------------------
|
|
|
|
There are a couple reasons that code in Django might be deprecated:
|
|
|
|
* If a feature has been improved or modified in a backwards-incompatible way,
|
|
the old feature or behavior will be deprecated.
|
|
|
|
* Sometimes Django will include a backport of a Python library that's not
|
|
included in a version of Python that Django currently supports. When Django
|
|
no longer needs to support the older version of Python that doesn't include
|
|
the library, the library will be deprecated in Django.
|
|
|
|
As the :ref:`deprecation policy<internal-release-deprecation-policy>` describes,
|
|
the first release of Django that deprecates a feature (``A.B``) should raise a
|
|
``RemovedInDjangoXXWarning`` (where XX is the Django version where the feature
|
|
will be removed) when the deprecated feature is invoked. Assuming
|
|
we have a good test coverage, these warnings will be shown by the test suite
|
|
when :ref:`running it <running-unit-tests>` with warnings enabled:
|
|
``python -Wall runtests.py``. This is annoying and the output of the test suite
|
|
should remain clean. Thus, when adding a ``RemovedInDjangoXXWarning`` you need
|
|
to eliminate or silence any warnings generated when running the tests.
|
|
|
|
The first step is to remove any use of the deprecated behavior by Django itself.
|
|
Next you can silence warnings in tests that actually test the deprecated
|
|
behavior in one of two ways:
|
|
|
|
#) In a particular test::
|
|
|
|
import warnings
|
|
|
|
def test_foo(self):
|
|
with warnings.catch_warnings(record=True) as w:
|
|
warnings.simplefilter("always")
|
|
# invoke deprecated behavior
|
|
# go ahead with the rest of the test
|
|
|
|
#) For an entire test case, ``django.test.utils`` contains three helpful
|
|
mixins to silence warnings: ``IgnorePendingDeprecationWarningsMixin``,
|
|
``IgnoreDeprecationWarningsMixin``, and
|
|
``IgnoreAllDeprecationWarningsMixin``. For example::
|
|
|
|
from django.test.utils import IgnorePendingDeprecationWarningsMixin
|
|
|
|
class MyDeprecatedTests(IgnorePendingDeprecationWarningsMixin, unittest.TestCase):
|
|
...
|
|
|
|
Finally, there are a couple of updates to Django's documentation to make:
|
|
|
|
#) If the existing feature is documented, mark it deprecated in documentation
|
|
using the ``.. deprecated:: A.B`` annotation. Include a short description
|
|
and a note about the upgrade path if applicable.
|
|
|
|
#) Add a description of the deprecated behavior, and the upgrade path if
|
|
applicable, to the current release notes (``docs/releases/A.B.txt``) under
|
|
the "Features deprecated in A.B" heading.
|
|
|
|
#) Add an entry in the deprecation timeline (``docs/internals/deprecation.txt``)
|
|
under the ``A.B+2`` version describing what code will be removed.
|
|
|
|
Once you have completed these steps, you are finished with the deprecation.
|
|
In each major release, all ``RemovedInDjangoXXWarning``\s matching the new
|
|
version are removed.
|
|
|
|
Javascript patches
|
|
------------------
|
|
|
|
Django's admin system leverages the jQuery framework to increase the
|
|
capabilities of the admin interface. In conjunction, there is an emphasis on
|
|
admin javascript performance and minimizing overall admin media file size.
|
|
Serving compressed or "minified" versions of javascript files is considered
|
|
best practice in this regard.
|
|
|
|
To that end, patches for javascript files should include both the original
|
|
code for future development (e.g. ``foo.js``), and a compressed version for
|
|
production use (e.g. ``foo.min.js``). Any links to the file in the codebase
|
|
should point to the compressed version.
|
|
|
|
Compressing JavaScript
|
|
~~~~~~~~~~~~~~~~~~~~~~
|
|
|
|
To simplify the process of providing optimized javascript code, Django
|
|
includes a handy python script which should be used to create a "minified"
|
|
version. To run it::
|
|
|
|
python django/contrib/admin/bin/compress.py
|
|
|
|
Behind the scenes, ``compress.py`` is a front-end for Google's
|
|
`Closure Compiler`_ which is written in Java. However, the Closure Compiler
|
|
library is not bundled with Django directly, so those wishing to contribute
|
|
complete javascript patches will need to download and install the library
|
|
independently.
|
|
|
|
The Closure Compiler library requires Java version 6 or higher (Java 1.6 or
|
|
higher on Mac OS X. Note that Mac OS X 10.5 and earlier did not ship with
|
|
Java 1.6 by default, so it may be necessary to upgrade your Java installation
|
|
before the tool will be functional. Also note that even after upgrading Java,
|
|
the default ``/usr/bin/java`` command may remain linked to the previous Java
|
|
binary, so relinking that command may be necessary as well.)
|
|
|
|
Please don't forget to run ``compress.py`` and include the ``diff`` of the
|
|
minified scripts when submitting patches for Django's javascript.
|
|
|
|
.. _Closure Compiler: https://developers.google.com/closure/compiler/
|
|
.. _list of tickets with patches: https://code.djangoproject.com/query?status=new&status=assigned&status=reopened&has_patch=1&order=priority
|
|
.. _ticket tracker: https://code.djangoproject.com/newticket
|