Fixed #28034 -- Updated the contributing tutorial to use an imaginary ticket.

This commit is contained in:
Adam Johnson 2018-09-19 13:54:44 +01:00 committed by Tim Graham
parent bdae19cf63
commit 18e4ade79e
1 changed files with 139 additions and 208 deletions

View File

@ -54,7 +54,7 @@ By the end of this tutorial, you should have a basic understanding of both the
tools and the processes involved. Specifically, we'll be covering the following:
* Installing Git.
* How to download a development copy of Django.
* Downloading a copy of Django's development version.
* Running Django's test suite.
* Writing a test for your patch.
* Writing the code for your patch.
@ -115,6 +115,12 @@ Download the Django source code repository using the following command:
$ git clone git@github.com:YourGitHubName/django.git
.. admonition:: Low bandwidth connection?
You can add the ``--depth 1`` argument to ``git clone`` to skip downloading
all of Django's commit history, which reduces data transfer from ~250 MB
to ~70 MB.
Now that you have a local copy of Django, you can install it just like you would
install any package using ``pip``. The most convenient way to do so is by using
a *virtual environment*, which is a feature built into Python that allows you
@ -179,12 +185,12 @@ When contributing to Django it's very important that your code changes don't
introduce bugs into other areas of Django. One way to check that Django still
works after you make your changes is by running Django's test suite. If all
the tests still pass, then you can be reasonably sure that your changes
haven't completely broken Django. If you've never run Django's test suite
before, it's a good idea to run it once beforehand just to get familiar with
what its output is supposed to look like.
work and haven't broken other parts Django. If you've never run Django's test
suite before, it's a good idea to run it once beforehand to get familiar with
its output.
Before running the test suite, install its dependencies by first ``cd``-ing
into the Django ``tests/`` directory and then running:
Before running the test suite, install its dependencies by ``cd``-ing into the
Django ``tests/`` directory and then running:
.. console::
@ -206,10 +212,10 @@ Now sit back and relax. Django's entire test suite has thousands of tests, and
it takes at least a few minutes run, depending on the speed of your computer.
While Django's test suite is running, you'll see a stream of characters
representing the status of each test as it's run. ``E`` indicates that an error
was raised during a test, and ``F`` indicates that a test's assertions failed.
Both of these are considered to be test failures. Meanwhile, ``x`` and ``s``
indicated expected failures and skipped tests, respectively. Dots indicate
representing the status of each test as it completes. ``E`` indicates that an
error was raised during a test, and ``F`` indicates that a test's assertions
failed. Both of these are considered to be test failures. Meanwhile, ``x`` and
``s`` indicated expected failures and skipped tests, respectively. Dots indicate
passing tests.
Skipped tests are typically due to missing external libraries required to run
@ -224,9 +230,7 @@ Once the tests complete, you should be greeted with a message informing you
whether the test suite passed or failed. Since you haven't yet made any changes
to Django's code, the entire test suite **should** pass. If you get failures or
errors make sure you've followed all of the previous steps properly. See
:ref:`running-unit-tests` for more information. There will be a couple failures
related to deprecation warnings that you can ignore. These failures have since
been fixed in Django.
:ref:`running-unit-tests` for more information.
Note that the latest Django master may not always be stable. When developing
against master, you can check `Django's continuous integration builds`__ to
@ -244,35 +248,18 @@ __ https://djangoci.com
:ref:`run the tests using a different database
<running-unit-tests-settings>`.
Rolling back to a previous revision of Django
=============================================
Working on a feature
====================
For this tutorial, we'll be using ticket :ticket:`24788` as a case study, so
we'll rewind Django's version history in git to before that ticket's patch was
applied. This will allow us to go through all of the steps involved in writing
that patch from scratch, including running Django's test suite.
For this tutorial, we'll work on a "fake ticket" as a case study. Here are the
imaginary details:
**Keep in mind that while we'll be using an older revision of Django for this
tutorial, you should always use the current version of the master branch when
working on your own patch for a ticket!**
.. admonition:: Ticket #99999 -- Allow making toast
.. note::
Django should provide a function ``django.shortcuts.make_toast()`` that
returns ``'toast'``.
The patch for this ticket was written by Paweł Marczewski, and it was
applied to Django as `commit 4df7e8483b2679fc1cba3410f08960bac6f51115`__.
Consequently, we'll be using the revision of Django just prior to that,
`commit 4ccfc4439a7add24f8db4ef3960d02ef8ae09887`__.
__ https://github.com/django/django/commit/4df7e8483b2679fc1cba3410f08960bac6f51115
__ https://github.com/django/django/commit/4ccfc4439a7add24f8db4ef3960d02ef8ae09887
Navigate into Django's root directory (that's the one that contains ``django``,
``docs``, ``tests``, ``AUTHORS``, etc.). You can then check out the older
revision of Django that we'll be using in the tutorial below:
.. console::
$ git checkout 4ccfc4439a7add24f8db4ef3960d02ef8ae09887
We'll now implement this feature and associated tests.
Creating a branch for your patch
================================
@ -281,9 +268,9 @@ Before making any changes, create a new branch for the ticket:
.. console::
$ git checkout -b ticket_24788
$ git checkout -b ticket_99999
You can choose any name that you want for the branch, "ticket_24788" is an
You can choose any name that you want for the branch, "ticket_99999" is an
example. All changes made in this branch will be specific to the ticket and
won't affect the main copy of the code that we cloned earlier.
@ -312,42 +299,25 @@ Now for our hands-on example.
__ https://en.wikipedia.org/wiki/Test-driven_development
Writing some tests for ticket #24788
------------------------------------
Writing a test for ticket #99999
--------------------------------
Ticket :ticket:`24788` proposes a small feature addition: the ability to
specify the class level attribute ``prefix`` on Form classes, so that::
In order to resolve this ticket, we'll add a ``make_toast()`` function to the
top-level ``django`` module. First we are going to write a test that tries to
use the function and check that its output looks correct.
[…] forms which ship with apps could effectively namespace themselves such
that N overlapping form fields could be POSTed at once and resolved to the
correct form.
Navigate to Django's ``tests/shortcuts/`` folder and create a new file
``test_make_toast.py``. Add the following code::
In order to resolve this ticket, we'll add a ``prefix`` attribute to the
``BaseForm`` class. When creating instances of this class, passing a prefix to
the ``__init__()`` method will still set that prefix on the created instance.
But not passing a prefix (or passing ``None``) will use the class-level prefix.
Before we make those changes though, we're going to write a couple tests to
verify that our modification functions correctly and continues to function
correctly in the future.
from django.shortcuts import make_toast
from django.test import SimpleTestCase
Navigate to Django's ``tests/forms_tests/tests/`` folder and open the
``test_forms.py`` file. Add the following code on line 1674 right before the
``test_forms_with_null_boolean`` function::
def test_class_prefix(self):
# Prefix can be also specified at the class level.
class Person(Form):
first_name = CharField()
prefix = 'foo'
class MakeToastTests(SimpleTestCase):
def test_make_toast(self):
self.assertEqual(make_toast(), 'toast')
p = Person()
self.assertEqual(p.prefix, 'foo')
p = Person(prefix='bar')
self.assertEqual(p.prefix, 'bar')
This new test checks that setting a class level prefix works as expected, and
that passing a ``prefix`` parameter when creating an instance still works too.
This test checks that the ``make_toast()`` returns ``'toast'``.
.. admonition:: But this testing thing looks kinda hard...
@ -367,67 +337,43 @@ __ http://www.diveintopython3.net/unit-testing.html
Running your new test
---------------------
Remember that we haven't actually made any modifications to ``BaseForm`` yet,
so our tests are going to fail. Let's run all the tests in the ``forms_tests``
folder to make sure that's really what happens. From the command line, ``cd``
into the Django ``tests/`` directory and run:
Since we haven't made any modifications to ``django.shortcuts`` yet, our test
should fail. Let's run all the tests in the ``shortcuts`` folder to make sure
that's really what happens. ``cd`` to the Django ``tests/`` directory and run:
.. console::
$ ./runtests.py forms_tests
$ ./runtests.py shortcuts
If the tests ran correctly, you should see one failure corresponding to the test
method we added. If all of the tests passed, then you'll want to make sure that
you added the new test shown above to the appropriate folder and class.
method we added, with this error::
ImportError: cannot import name 'make_toast' from 'django.shortcuts'
If all of the tests passed, then you'll want to make sure that you added the
new test shown above to the appropriate folder and file name.
Writing the code for your ticket
================================
Next we'll be adding the functionality described in ticket :ticket:`24788` to
Django.
Next we'll be adding the ``make_toast()`` function.
Writing the code for ticket #24788
----------------------------------
Navigate to the ``django/`` folder and open the ``shortcuts.py`` file.
Add the bottom, add the function::
Navigate to the ``django/django/forms/`` folder and open the ``forms.py`` file.
Find the ``BaseForm`` class on line 72 and add the ``prefix`` class attribute
right after the ``field_order`` attribute::
def make_toast():
return 'toast'
class BaseForm:
# This is the main implementation of all the Form logic. Note that this
# class is different than Form. See the comments by the Form class for
# more information. Any improvements to the form API should be made to
# *this* class, not to the Form class.
field_order = None
prefix = None
Verifying your test now passes
------------------------------
Once you're done modifying Django, we need to make sure that the tests we wrote
earlier pass, so we can see whether the code we wrote above is working
correctly. To run the tests in the ``forms_tests`` folder, ``cd`` into the
Django ``tests/`` directory and run:
Now we need to make sure that the test we wrote earlier passes, so we can see
whether the code we added is working correctly. Again, navigate to the Django
``tests/`` directory and run:
.. console::
$ ./runtests.py forms_tests
$ ./runtests.py shortcuts
Oops, good thing we wrote those tests! You should still see one failure with
the following exception::
AssertionError: None != 'foo'
We forgot to add the conditional statement in the ``__init__`` method. Go ahead
and change ``self.prefix = prefix`` that is now on line 87 of
``django/forms/forms.py``, adding a conditional statement::
if prefix is not None:
self.prefix = prefix
Re-run the tests and everything should pass. If it doesn't, make sure you
correctly modified the ``BaseForm`` class as shown above and copied the new test
correctly.
Everything should pass. If it doesn't, make sure you correctly added the
function to the correct file.
Running Django's test suite for the second time
===============================================
@ -445,32 +391,29 @@ directory and run:
$ ./runtests.py
Remember that for this tutorial you're working from an older version of Django.
You may see a few unrelated failures that have since been fixed in Django's
master branch.
Writing Documentation
=====================
This is a new feature, so it should be documented. Add the following section on
line 1068 (at the end of the file) of ``django/docs/ref/forms/api.txt``::
This is a new feature, so it should be documented. Open the file
``docs/topics/http/shortcuts.txt`` and add the following at the end of the
file::
The prefix can also be specified on the form class::
``make_toast()``
================
>>> class PersonForm(forms.Form):
... ...
... prefix = 'person'
.. versionadded:: 2.2
.. versionadded:: 1.9
The ability to specify ``prefix`` on the form class was added.
Returns ``'toast'``.
Since this new feature will be in an upcoming release it is also added to the
release notes for Django 1.9, on line 164 under the "Forms" section in the file
``docs/releases/1.9.txt``::
release notes for the next version of Django. Open the release notes for the
latest version in ``docs/releases/``, which at time of writing is ``2.2.txt``.
Add a note under the "Minor Features" header::
* A form prefix can be specified inside a form class, not only when
instantiating a form. See :ref:`form-prefix` for details.
:mod:`django.shortcuts`
~~~~~~~~~~~~~~~~~~~~~~~
* The new :func:`django.shortcuts.make_toast` function returns ``'toast'``.
For more information on writing documentation, including an explanation of what
the ``versionadded`` bit is all about, see
@ -481,95 +424,83 @@ preview the HTML that will be generated.
Previewing your changes
=======================
Now it's time to go through all the changes made in our patch. To display the
differences between your current copy of Django (with your changes) and the
revision that you initially checked out earlier in the tutorial:
Now it's time to go through all the changes made in our patch. To stage all the
changes ready for commit, run:
.. console::
$ git diff
$ git add --all
Then display the differences between your current copy of Django (with your
changes) and the revision that you initially checked out earlier in the
tutorial with:
.. console::
$ git diff --cached
Use the arrow keys to move up and down.
.. code-block:: diff
diff --git a/django/forms/forms.py b/django/forms/forms.py
index 509709f..d1370de 100644
--- a/django/forms/forms.py
+++ b/django/forms/forms.py
@@ -75,6 +75,7 @@ class BaseForm:
# information. Any improvements to the form API should be made to *this*
# class, not to the Form class.
field_order = None
+ prefix = None
diff --git a/django/shortcuts.py b/django/shortcuts.py
index 7ab1df0e9d..8dde9e28d9 100644
--- a/django/shortcuts.py
+++ b/django/shortcuts.py
@@ -156,3 +156,7 @@ def resolve_url(to, *args, **kwargs):
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=None,
@@ -83,7 +84,8 @@ class BaseForm:
self.data = data or {}
self.files = files or {}
self.auto_id = auto_id
- self.prefix = prefix
+ if prefix is not None:
+ self.prefix = prefix
self.initial = initial or {}
self.error_class = error_class
# Translators: This is the default suffix added to form field labels
diff --git a/docs/ref/forms/api.txt b/docs/ref/forms/api.txt
index 3bc39cd..008170d 100644
--- a/docs/ref/forms/api.txt
+++ b/docs/ref/forms/api.txt
@@ -1065,3 +1065,13 @@ You can put several Django forms inside one ``<form>`` tag. To give each
>>> print(father.as_ul())
<li><label for="id_father-first_name">First name:</label> <input type="text" name="father-first_name" id="id_father-first_name"></li>
<li><label for="id_father-last_name">Last name:</label> <input type="text" name="father-last_name" id="id_father-last_name"></li>
# Finally, fall back and assume it's a URL
return to
+
+The prefix can also be specified on the form class::
+
+ >>> class PersonForm(forms.Form):
+ ... ...
+ ... prefix = 'person'
+
+.. versionadded:: 1.9
+
+ The ability to specify ``prefix`` on the form class was added.
diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt
index 5b58f79..f9bb9de 100644
--- a/docs/releases/1.9.txt
+++ b/docs/releases/1.9.txt
@@ -161,6 +161,9 @@ Forms
:attr:`~django.forms.Form.field_order` attribute, the ``field_order``
constructor argument , or the :meth:`~django.forms.Form.order_fields` method.
+def make_toast():
+ return 'toast'
diff --git a/docs/releases/2.2.txt b/docs/releases/2.2.txt
index 7d85d30c4a..81518187b3 100644
--- a/docs/releases/2.2.txt
+++ b/docs/releases/2.2.txt
@@ -40,6 +40,11 @@ database constraints. Constraints are added to models using the
Minor features
--------------
+* A form prefix can be specified inside a form class, not only when
+ instantiating a form. See :ref:`form-prefix` for details.
+:mod:`django.shortcuts`
+~~~~~~~~~~~~~~~~~~~~~~~
+
Generic Views
^^^^^^^^^^^^^
+* The new :func:`django.shortcuts.make_toast` function returns ``'toast'``.
+
:mod:`django.contrib.admin`
~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py
index 690f205..e07fae2 100644
--- a/tests/forms_tests/tests/test_forms.py
+++ b/tests/forms_tests/tests/test_forms.py
@@ -1671,6 +1671,18 @@ class FormsTestCase(SimpleTestCase):
self.assertEqual(p.cleaned_data['last_name'], 'Lennon')
self.assertEqual(p.cleaned_data['birthday'], datetime.date(1940, 10, 9))
+ def test_class_prefix(self):
+ # Prefix can be also specified at the class level.
+ class Person(Form):
+ first_name = CharField()
+ prefix = 'foo'
diff --git a/docs/topics/http/shortcuts.txt b/docs/topics/http/shortcuts.txt
index 7b3a3a2c00..711bf6bb6d 100644
--- a/docs/topics/http/shortcuts.txt
+++ b/docs/topics/http/shortcuts.txt
@@ -271,3 +271,12 @@ This example is equivalent to::
my_objects = list(MyModel.objects.filter(published=True))
if not my_objects:
raise Http404("No MyModel matches the given query.")
+
+ p = Person()
+ self.assertEqual(p.prefix, 'foo')
+``make_toast()``
+================
+
+ p = Person(prefix='bar')
+ self.assertEqual(p.prefix, 'bar')
+.. function:: make_toast()
+
def test_forms_with_null_boolean(self):
# NullBooleanField is a bit of a special case because its presentation (widget)
# is different than its data. This is handled transparently, though.
+.. versionadded:: 2.2
+
+Returns ``'toast'``.
diff --git a/tests/shortcuts/test_make_toast.py b/tests/shortcuts/test_make_toast.py
new file mode 100644
index 0000000000..6f4c627b6e
--- /dev/null
+++ b/tests/shortcuts/test_make_toast.py
@@ -0,0 +1,7 @@
+from django.shortcuts import make_toast
+from django.test import SimpleTestCase
+
+
+class MakeToastTests(SimpleTestCase):
+ def test_make_toast(self):
+ self.assertEqual(make_toast(), 'toast')
When you're done previewing the patch, hit the ``q`` key to return to the
command line. If the patch's content looked okay, it's time to commit the
@ -582,24 +513,24 @@ To commit the changes:
.. console::
$ git commit -a
$ git commit
This opens up a text editor to type the commit message. Follow the :ref:`commit
message guidelines <committing-guidelines>` and write a message like:
.. code-block:: text
Fixed #24788 -- Allowed Forms to specify a prefix at the class level.
Fixed #99999 -- Added a shortcut function to make toast.
Pushing the commit and making a pull request
============================================
After committing the patch, send it to your fork on GitHub (substitute
"ticket_24788" with the name of your branch if it's different):
"ticket_99999" with the name of your branch if it's different):
.. console::
$ git push origin ticket_24788
$ git push origin ticket_99999
You can create a pull request by visiting the `Django GitHub page
<https://github.com/django/django/>`_. You'll see your branch under "Your