mirror of
https://github.com/django/django.git
synced 2024-12-24 10:05:46 +00:00
314 lines
13 KiB
Plaintext
314 lines
13 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.
|
|
|
|
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:
|
|
|
|
* `Login using your GitHub account`_ or `create an account`_ 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 may be working on this ticket. Either find another
|
|
bug/feature to work on, or contact the developer working on the ticket to
|
|
offer your help. If a ticket has been assigned for weeks or months without
|
|
any activity, it's probably safe to reassign it to yourself.
|
|
|
|
* Log into your account, if you haven't already, by clicking "GitHub Login"
|
|
or "DjangoProject Login" in the upper left of the ticket page.
|
|
|
|
* Claim the ticket by clicking the "assign to myself" radio button under
|
|
"Action" near the bottom of the page, 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.
|
|
|
|
.. _Login using your GitHub account: https://code.djangoproject.com/github/login
|
|
.. _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.
|
|
|
|
When you think your work is ready to be reviewed, send :doc:`a GitHub pull
|
|
request <working-with-git>`. Please review the patch yourself using our
|
|
:ref:`patch review checklist <patch-review-checklist>` first.
|
|
|
|
If you can't send a pull request for some reason, 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.
|
|
|
|
* 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 fulfills the requirements in our :ref:`patch review
|
|
checklist <patch-review-checklist>`.
|
|
|
|
* Check the "Has patch" box on the ticket and make sure the "Needs
|
|
documentation", "Needs tests", and "Patch needs improvement" boxes aren't
|
|
checked. This makes the ticket appear in the "Patches needing review" queue
|
|
on the `Development dashboard`_.
|
|
|
|
.. _ticket tracker: https://code.djangoproject.com/
|
|
.. _Development dashboard: https://dashboard.djangoproject.com/
|
|
|
|
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:
|
|
|
|
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 good
|
|
test coverage, these warnings are converted to errors when :ref:`running the
|
|
test suite <running-unit-tests>` with warnings enabled:
|
|
``python -Wa runtests.py``. 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 by using the ``ignore_warnings`` decorator, either at the test or class
|
|
level:
|
|
|
|
#) In a particular test::
|
|
|
|
from django.test import ignore_warnings
|
|
from django.utils.deprecation import RemovedInDjangoXXWarning
|
|
|
|
@ignore_warnings(category=RemovedInDjangoXXWarning)
|
|
def test_foo(self):
|
|
...
|
|
|
|
#) For an entire test case::
|
|
|
|
from django.test import ignore_warnings
|
|
from django.utils.deprecation import RemovedInDjangoXXWarning
|
|
|
|
@ignore_warnings(category=RemovedInDjangoXXWarning)
|
|
class MyDeprecatedTests(unittest.TestCase):
|
|
...
|
|
|
|
You can also add a test for the deprecation warning. You'll have to disable the
|
|
"warning as error" behavior in your test by doing::
|
|
|
|
import warnings
|
|
|
|
def test_foo_deprecation_warning(self):
|
|
with warnings.catch_warnings(record=True) as warns:
|
|
warnings.simplefilter('always') # prevent warnings from appearing as errors
|
|
# invoke deprecated behavior
|
|
|
|
self.assertEqual(len(warns), 1)
|
|
msg = str(warns[0].message)
|
|
self.assertEqual(msg, 'Expected deprecation message')
|
|
|
|
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 appropriate version describing what code will be removed.
|
|
|
|
Once you have completed these steps, you are finished with the deprecation.
|
|
In each :term:`feature release`, all ``RemovedInDjangoXXWarning``\s matching
|
|
the new version are removed.
|
|
|
|
JavaScript patches
|
|
==================
|
|
|
|
For information on JavaScript patches, see the :ref:`javascript-patches`
|
|
documentation.
|
|
|
|
.. _patch-review-checklist:
|
|
|
|
Patch review checklist
|
|
======================
|
|
|
|
Use this checklist to review a pull request. If you are reviewing a pull
|
|
request that is not your own and it passes all the criteria below, please set
|
|
the "Triage Stage" on the corresponding Trac ticket to "Ready for checkin".
|
|
If you've left comments for improvement on the pull request, please tick the
|
|
appropriate flags on the Trac ticket based on the results of your review:
|
|
"Patch needs improvement", "Needs documentation", and/or "Needs tests". As time
|
|
and interest permits, committers do final reviews of "Ready for checkin"
|
|
tickets and will either commit the patch or bump it back to "Accepted" if
|
|
further works need to be done. If you're looking to become a committer,
|
|
doing thorough reviews of patches is a great way to earn trust.
|
|
|
|
Looking for a patch to review? Check out the "Patches needing review" section
|
|
of the `Django Development Dashboard <https://dashboard.djangoproject.com/>`_.
|
|
Looking to get your patch reviewed? Ensure the Trac flags on the ticket are
|
|
set so that the ticket appears in that queue.
|
|
|
|
Documentation
|
|
-------------
|
|
|
|
* Does the documentation build without any errors (``make html``, or
|
|
``make.bat html`` on Windows, from the ``docs`` directory)?
|
|
* Does the documentation follow the writing style guidelines in
|
|
:doc:`/internals/contributing/writing-documentation`?
|
|
* Are there any :ref:`spelling errors <documentation-spelling-check>`?
|
|
|
|
Bugs
|
|
----
|
|
|
|
* Is there a proper regression test (the test should fail before the fix
|
|
is applied)?
|
|
* If it's a bug that :ref:`qualifies for a backport <supported-versions-policy>`
|
|
to the stable version of Django, is there a release note in
|
|
``docs/releases/A.B.C.txt``? Bug fixes that will be applied only to the
|
|
master branch don't need a release note.
|
|
|
|
New Features
|
|
------------
|
|
|
|
* Are there tests to "exercise" all of the new code?
|
|
* Is there a release note in ``docs/releases/A.B.txt``?
|
|
* Is there documentation for the feature and is it :ref:`annotated
|
|
appropriately <documenting-new-features>` with
|
|
``.. versionadded:: A.B`` or ``.. versionchanged:: A.B``?
|
|
|
|
Deprecating a feature
|
|
---------------------
|
|
|
|
See the :ref:`deprecating-a-feature` guide.
|
|
|
|
All code changes
|
|
----------------
|
|
|
|
* Does the :doc:`coding style
|
|
</internals/contributing/writing-code/coding-style>` conform to our
|
|
guidelines? Are there any ``flake8`` errors?
|
|
* If the change is backwards incompatible in any way, is there a note
|
|
in the release notes (``docs/releases/A.B.txt``)?
|
|
* Is Django's test suite passing?
|
|
|
|
All tickets
|
|
-----------
|
|
|
|
* Is the pull request a single squashed commit with a message that follows our
|
|
:ref:`commit message format <committing-guidelines>`?
|
|
* Are you the patch author and a new contributor? Please add yourself to the
|
|
``AUTHORS`` file and submit a `Contributor License Agreement`_.
|
|
|
|
.. _Contributor License Agreement: https://www.djangoproject.com/foundation/cla/
|