Ticket #16779: contrib-tutorial-2.txt

File contrib-tutorial-2.txt, 24.0 KB (added by taavi223, 4 years ago)

Removed Trac instructions in favor of linking to that documentation

Line 
1===================================
2Writing your first patch for Django
3===================================
4
5Now that you've finished writing your first Django app, how about giving back to the community a little? Maybe you've found a bug in Django that you'd like to see fixed, or maybe there's a small feature you want added.
6
7Contributing back to Django itself is the best way to see your own concerns addressed. This may seem daunting at first, but it's actually pretty simple. We'll walk you through the entire process, so you can learn by example.
8
9In this tutorial, we'll walk through contributing a patch to Django for the first time. By the end of this tutorial, you should understand both the tools and the process involved. We'll be covering the following:
10
11    * The tools you'll need to get started.
12    * How to write your first patch.
13    * Where to find more information.
14   
15Once you're done with the tutorial, you can look through the rest of :doc:`Django's documentation on contributing<internals/contributing/index>`. It contains lots of great information and is a must read for anyone who'd like to become a regular contributor to Django. If you've got questions, it's probably got the answers.
16
17.. admonition:: Where to get help:
18
19    If you're having trouble going through this tutorial, please post a message
20    to `django-users`__ or drop by `#django on irc.freenode.net`__ to chat
21    with other Django users who might be able to help.
22
23__ https://code.djangoproject.com/ticket/15315
24__ https://code.djangoproject.com/changeset/16659
25__ http://groups.google.com/group/django-users
26__ irc://irc.freenode.net/django
27
28What you'll need to get started
29===============================
30
31Subversion or Git
32-----------------
33
34For this tutorial, you'll need to have either Subversion or Git installed. These tools will be used to download the current development version of Django and to generate patch files for the changes you make.
35
36To check whether or not you have one of these tools already installed, enter either ``svn`` or ``git`` into the command line. If you get messages saying that neither of these commands could be found, you'll have to download and install one of them yourself.
37
38You can download and install Subversion from the `Apache's Subversion Binary Packages page`__.
39
40Alternatively, you can download and install Git from `Git's download page`__.
41
42__ http://subversion.apache.org/packages.html
43__ http://git-scm.com/download
44
45How to write your first patch
46=============================
47
48Checking out Django's development version
49-----------------------------------------
50
51The first step to contributing to Django is to check out the Django's current development revision using either Subversion or Git.
52
53.. note::
54   
55    For this tutorial, we'll be using `ticket 15315`__ as a case study, so we'll be using an older version of Django from before this 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. Keep in mind that while we'll be checking out an **older** revision of Django's trunk below, you should always check out the **current** development revision of Django when working on your own patch for another ticket.
56       
57    The patch for this ticket was generously written by SardarNL and Will Hardy, and it was applied to Django as `changeset 16659`__. Consequently, we'll be checking out the revision just prior to that, revision 16658.
58
59From the command line, use the ``cd`` command to navigate to the directory where you'll want your local copy of Django to live.
60
61Using Subversion
62~~~~~~~~~~~~~~~~
63
64If you have Subversion installed, you can run the following command to download revision 16658 of Django:
65
66.. code-block:: bash
67   
68    svn checkout -r 16658 http://code.djangoproject.com/svn/django/trunk/
69
70Using Git
71~~~~~~~~~
72
73Alternatively, if you prefer using Git, you can clone the official Django Git mirror (which is updated from the Subversion repository every five minutes), and then switch to revision 16658 as shown below:
74
75.. code-block:: bash
76
77    git clone https://github.com/django/django.git
78    cd django
79    git checkout 90e8bd48f2
80
81.. note::
82
83    If you're not that familiar with Subversion or Git, you can always find out more about their various commands by typing either ``svn help`` or ``git help`` into the command line.
84
85Running Django's test suite
86---------------------------
87
88When 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 stills works after you make your changes is by running Django's entire 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.
89
90
91Setting Django up to run the test suite
92~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
93
94Before we can actually run the test suite though, we need to make sure that your new local copy of Django is on your ``PYTHONPATH``; otherwise, the test suite won't run properly. We also need to make sure that there aren't any **other** copies of Django installed somewhere else that are taking priority over your newer copy (this happens more often than you might think). To check for these problems, start up the Python interpreter and follow along with the code below:
95
96.. code-block:: python
97   
98    >>> import django
99    >>> django
100    <module 'django' from '/.../django/__init__.pyc'>
101
102If you get an ``ImportError: No module named django`` after entering the first line, then you'll need to add your new copy of Django to your ``PYTHONPATH``. For more details on how to do this, read :ref:`pointing-python-at-the-new-django-version`.
103
104If you didn't get any errors, then look at the path found in the third line (abbreviated above as ``/.../django/__init__.pyc``). If that isn't the directory that you put Django into earlier in this tutorial, then there is **another** copy of Django on your ``PYTHONPATH`` that is taking priority over the newer copy.  You'll either have to remove this older copy from your ``PYTHONPATH``, or add your new copy to the beginning of your ``PYTHONPATH`` so that it takes priority.
105
106.. note::
107
108    If you're a savvy Djangonaut you might be thinking that using ``virtualenv`` would work perfectly for this type of thing, and you'd be right (+100 bonus points). Using ``virtualenv`` with the ``--no-site-packages`` option isolates your local copy of Django from the rest of your system and avoids potential conflicts. Just make sure not to use ``pip`` to install Django, since that causes Django's setup.py script to be run. You'll still need to use Subversion or Git to check out a copy of Django, and then :ref:`manually add it to your ``PYTHONPATH``<pointing-python-at-the-new-django-version>`.
109
110Actually running the Django test suite
111~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
112
113Once Django is setup properly, we can actually run the test suite. Simply ``cd`` into the Django ``tests/`` directory and run:
114
115.. code-block:: bash
116
117    ./runtests.py --settings=test_sqlite
118
119If you get an ``ImportError: No module named django.contrib`` error, you still need to add your current copy of Django to your ``PYTHONPATH``. For more details on how to do this, read :ref:`pointing-python-at-the-new-django-version`.
120
121Otherwise, sit back and relax. Django's entire test suite has over 4000 different tests, so it can take anywhere from 5 to 15 minutes to run, depending on the speed of your computer.
122
123.. note::
124   
125    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 indicated passing tests.
126
127Once the process is done, 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 it doesn't, make sure you've followed all of the previous steps properly.
128
129More information on how to run Django's test suite can be found in the :doc:`online documentation</internals/contributing/writing-code/unit-tests>`.
130
131Writing a test for your ticket
132------------------------------
133
134In most cases, for a patch to be accepted into Django, it has to include tests. For bug fix patches, this means writing a regression test to ensure that the bug is never reintroduced into Django later on. A regression test should be written in such a way that it will fail while the bug still exists and pass once the bug has been fixed. For patches containing new features, you'll need to include tests which ensure that the new features are working correctly. They too should fail when the new feature is not present, and then pass once it has been implemented.
135
136A good way to do this is to write your new tests first before making any changes to the code, and then run them to ensure that they do indeed fail. If your new tests don't fail, you'll need to fix them so that they do. After all, a regression test that passes regardless of whether a bug is present is not very helpful for preventing that bug's reoccurrence.
137
138Now for our hands on example.
139
140Writing a test for ticket 15315
141~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
142
143`Ticket 15315`__ describes the following, small feature addition:
144   
145    Since Django 1.2 model forms can override widgets by specifying a ``'widgets'`` attribute in ``Meta``, similar to ``'fields'`` or ``'exclude'``. The ``modelform_factory`` function doesn't accept widgets, so the only way to specify it is by defining a new parent ``ModelForm`` with ``Meta`` and giving it as the ``'form'`` argument. This is more complex than it needs to be.
146
147    The fix is to add a new keyword argument ``widgets=None`` to ``modelform_factory``.
148   
149In order to resolve this ticket, we'll modify the ``modelform_factory`` function to accept a ``widgets`` keyword argument. Before we make those changes though, we're going to write a test to verify that our modification actually functions correctly and continues to function correctly in the future.
150
151Navigate to the Django's ``tests/regressiontests/model_forms_regress/`` folder and open the ``tests.py`` file. Find the :class:`FormFieldCallbackTests` class on line 264 and add the ``test_factory_with_widget_argument`` test to it as shown below:
152
153.. code-block:: python
154   
155    class FormFieldCallbackTests(TestCase):
156   
157        def test_factory_with_widget_argument(self):
158            """ Regression for #15315: modelform_factory should accept widgets
159                argument
160            """
161            widget = forms.Textarea()
162           
163            # Without the widget, the form field should not have the widget set to a textarea
164            Form = modelform_factory(Person)
165            self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
166           
167            # With a widget, the form field should have the widget set to a textarea
168            Form = modelform_factory(Person, widgets={'name':widget})
169            self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
170       
171        def test_baseform_with_widgets_in_meta(self):
172            ...
173
174This test checks to see if the function ``modelform_factory`` accepts the new widgets argument specifying what widgets to use for each field. It also makes sure that those form fields actually use the specified widgets. Now we have the test for our patch written.
175
176__ https://code.djangoproject.com/ticket/15315
177
178Running your new test
179~~~~~~~~~~~~~~~~~~~~~
180
181Remember that we haven't actually made any modifications to ``modelform_factory`` yet, so our test is going to fail.  Let's run all the tests in the ``model_forms_regress`` folder to make sure that's really what happens. From the command line, ``cd`` into the Django ``tests/`` directory and run:
182
183.. code-block:: bash
184
185    ./runtests.py --settings=test_sqlite model_forms_regress
186
187If tests ran correctly, you should see that one of the tests failed with an error. Verify that it was the new ``test_factory_with_widget_argument`` test we added above, and then go on to the next step.
188
189If 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. It's also possible that you have a second copy of Django on your ``PYTHONPATH`` that is taking priority over this copy, and thus it may actually be that copy of Django whose tests are being run. To check if this might be the problem, refer to the section `Setting Django up to run the test suite`_.
190
191Making your changes to Django
192-----------------------------
193
194Next we'll actually add the functionality described in `ticket 15315`__ to Django.
195
196Fixing ticket 1315
197~~~~~~~~~~~~~~~~~~
198
199Navigate to the ``trunk/django/forms/`` folder and open ``models.py``. Find the ``modelform_factory`` function on line 370 and modify it so that it reads as follows:
200
201.. code-block:: python
202
203    def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
204                          formfield_callback=None,  widgets=None):
205        # Create the inner Meta class. FIXME: ideally, we should be able to
206        # construct a ModelForm without creating and passing in a temporary
207        # inner class.
208     
209        # Build up a list of attributes that the Meta object will have.
210        attrs = {'model': model}
211        if fields is not None:
212            attrs['fields'] = fields
213        if exclude is not None:
214            attrs['exclude'] = exclude
215        if widgets is not None:
216            attrs['widgets'] = widgets
217     
218        # If parent form class already has an inner Meta, the Meta we're
219        # creating needs to inherit from the parent's inner meta.
220        parent = (object,)
221        if hasattr(form, 'Meta'):
222            parent = (form.Meta, object)
223        Meta = type('Meta', parent, attrs)
224     
225        # Give this new form class a reasonable name.
226        class_name = model.__name__ + 'Form'
227     
228        # Class attributes for the new form class.
229        form_class_attrs = {
230            'Meta': Meta,
231            'formfield_callback': formfield_callback
232        }
233     
234        form_metaclass = ModelFormMetaclass
235     
236        if issubclass(form, BaseModelForm) and hasattr(form, '__metaclass__'):
237            form_metaclass = form.__metaclass__
238     
239        return form_metaclass(class_name, (form,), form_class_attrs)
240   
241Notice that we're only actually adding three new lines of code. We've added a new ``widgets`` keyword argument to the function's signature, and then two more lines which set ``attrs['widgets']`` to the value of that argument whenever it's supplied.
242
243Verifying your test now passes
244~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
245
246Once you're done modifying ``modelform_factory``, we need to make sure that the test we wrote earlier passes now. To run the tests in the ``model_forms_regress`` folder, ``cd`` into the Django ``tests/`` directory and run:
247
248.. code-block:: bash
249
250    ./runtests.py --settings=test_sqlite model_forms_regress
251
252If everything passes, then we can move on. If it doesn't, make sure you correctly modified the ``modelform_factory`` function as shown above and copied the ``test_factory_with_widget_argument`` test correctly.
253
254__ https://code.djangoproject.com/ticket/15315
255
256Running Django's test suite (again)
257-----------------------------------
258
259Once you've verified that your patch and your test work correctly, it's a good idea to run the entire Django test suite to verify that your change hasn't introduced any bugs in other areas of Django. While successfully passing the entire test suite doesn't guarantee your code is bug free, it does help identify many bugs and regressions which otherwise would go unnoticed.
260
261To run the entire Django test suite, ``cd`` into the Django ``tests/`` directory and run:
262
263.. code-block:: bash
264
265    ./runtests.py --settings=test_sqlite
266
267As long as you don't see any failures, you're good to go, and you can move on to generating a patch file that can be uploaded to Trac. If you do have any failures, you'll need to examine them in order to determine whether or not the modifications you made caused them.
268
269Generating a patch file
270-----------------------
271
272Now it's time to actually generate a patch file that can be uploaded to Trac or applied to another copy of Django.
273
274First, you'll need to navigate to your root Django directory (that's the one that contains ``django``, ``docs``, ``tests``, ``AUTHORS``, etc.). This is where we'll generate the patch file from.
275
276Creating a patch file with Subversion
277~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
278
279If you used Subversion to checkout your copy of Django, you can get a look at what your patch file will look like by running the following command:
280
281.. code-block:: bash
282
283    svn diff
284   
285This will display the differences between your current copy of Django (with your changes) and the revision that you initially checked out earlier in the tutorial. It should look similar to (but not necessarily the same as) the following:
286
287.. code-block:: diff
288
289    Index: django/forms/models.py
290    ===================================================================
291    --- django/forms/models.py  (revision 16658)
292    +++ django/forms/models.py  (working copy)
293    @@ -368,7 +368,7 @@
294         __metaclass__ = ModelFormMetaclass
295     
296     def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
297    -                       formfield_callback=None):
298    +                       formfield_callback=None, widgets=None):
299         # Create the inner Meta class. FIXME: ideally, we should be able to
300         # construct a ModelForm without creating and passing in a temporary
301         # inner class.
302    @@ -379,7 +379,9 @@
303             attrs['fields'] = fields
304         if exclude is not None:
305             attrs['exclude'] = exclude
306    -
307    +    if widgets is not None:
308    +        attrs['widgets'] = widgets
309    +   
310         # If parent form class already has an inner Meta, the Meta we're
311         # creating needs to inherit from the parent's inner meta.
312         parent = (object,)
313   
314    Index: tests/regressiontests/model_forms_regress/tests.py
315    ===================================================================
316    --- tests/regressiontests/model_forms_regress/tests.py      (revision 16658)
317    +++ tests/regressiontests/model_forms_regress/tests.py      (working copy)
318    @@ -263,6 +263,20 @@
319     
320     class FormFieldCallbackTests(TestCase):
321     
322    +    def test_factory_with_widget_argument(self):
323    +        """ Regression for #15315: modelform_factory should accept widgets
324    +            argument
325    +        """
326    +        widget = forms.Textarea()
327    +       
328    +        # Without a widget should not set the widget to textarea
329    +        Form = modelform_factory(Person)
330    +        self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
331    +       
332    +        # With a widget should not set the widget to textarea
333    +        Form = modelform_factory(Person, widgets={'name':widget})
334    +        self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
335    +
336         def test_baseform_with_widgets_in_meta(self):
337             """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors."""
338             widget = forms.Textarea()
339
340If the patch's content looks okay, you can run the following command to save the patch file to your current working directory.
341
342.. code-block:: bash
343
344    svn diff > myfancynewpatch.diff
345
346You should now have a file in the root Django directory called ``myfancynewpatch.diff``. This patch file that contains all your changes.
347
348Creating a patch file with Git
349~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
350
351For those who prefer using Git, the process is much the same as with Subversion.  To get a look at the content of your patch, run the following command instead:
352
353.. code-block:: bash
354
355    git diff
356   
357This will display the differences between your current copy of Django (with your changes) and the revision that you initially checked out earlier in the tutorial. It should look similar to (but not necessarily the same as) the following:
358   
359.. code-block:: diff
360
361    diff --git a/django/forms/models.py b/django/forms/models.py
362    index 527da5e..a93f21c 100644
363    --- a/django/forms/models.py
364    +++ b/django/forms/models.py
365    @@ -368,7 +368,7 @@ class ModelForm(BaseModelForm):
366         __metaclass__ = ModelFormMetaclass
367     
368     def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
369    -                       formfield_callback=None):
370    +                       formfield_callback=None, widgets=None):
371         # Create the inner Meta class. FIXME: ideally, we should be able to
372         # construct a ModelForm without creating and passing in a temporary
373         # inner class.
374    @@ -379,6 +379,8 @@ def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
375             attrs['fields'] = fields
376         if exclude is not None:
377             attrs['exclude'] = exclude
378    +    if widgets is not None:
379    +        attrs['widgets'] = widgets
380     
381         # If parent form class already has an inner Meta, the Meta we're
382         # creating needs to inherit from the parent's inner meta.
383   
384    diff --git a/tests/regressiontests/model_forms_regress/tests.py b/tests/regressiontests/model_forms_regress/tests
385    index 9860c2e..d2d9aa3 100644
386    --- a/tests/regressiontests/model_forms_regress/tests.py
387    +++ b/tests/regressiontests/model_forms_regress/tests.py
388    @@ -263,6 +263,20 @@ class URLFieldTests(TestCase):
389     
390     class FormFieldCallbackTests(TestCase):
391     
392    +    def test_factory_with_widget_argument(self):
393    +        """ Regression for #15315: modelform_factory should accept widgets
394    +            argument
395    +        """
396    +        widget = forms.Textarea()
397    +       
398    +        # Without a widget should not set the widget to textarea
399    +        Form = modelform_factory(Person)
400    +        self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
401    +       
402    +        # With a widget should not set the widget to textarea
403    +        Form = modelform_factory(Person, widgets={'name':widget})
404    +        self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
405    +
406         def test_baseform_with_widgets_in_meta(self):
407             """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors."""
408             widget = forms.Textarea()
409
410The Git patch format is slightly different than the Subversion format, but both are acceptable for patches.  Once you're done looking at the patch, hit the ``Q`` key to exit back to the command line.  If the patch's content looked okay, you can run the following command to save the patch file to your current working directory.
411
412.. code-block:: bash
413
414    git diff > myfancynewpatch.diff
415
416You should now have a file in the root Django directory called ``myfancynewpatch.diff``. This patch file that contains all your changes.
417
418Where to find more information
419==============================
420
421Now that you've generated your first patch, you're ready to go on and find a ticket of your own to write a patch for. Before you do that, make sure you read Django's documentation on :doc:`claiming tickets and submitting patches</internals/contributing/writing-code/submitting-patches>`. It covers how to claim tickets on Trac, expected coding style for patches, and many other important details.
422
423First time contributors should probably also read Django's :doc:`documentation for first time contributors</internals/contributing/new-contributors/>`. It has lots of good advice for those of us who are new to helping with Django.
424
425After those, if you're still hungering for more information about contributing, you can always browse through the rest of :doc:`Django's documentation on contributing<internals/contributing/index>`. It contains a ton of useful information and should be your first source for answering any questions you might have.
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
Back to Top