Code

Ticket #16779: contrib-tutorial.txt

File contrib-tutorial.txt, 27.6 KB (added by taavi223, 3 years ago)
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    * What you'll need to get started.
12    * How to write your first patch.
13    * Working together with the community.
14
15.. note::
16   
17    For this tutorial, we'll be using `ticket 15315`__ as a case study. As part of this process, we will be going through all of the steps involved in writing that patch from scratch, including running Django's test suite. In order to do this though, we'll be working with an older revision of Django in this tutorial. When working on your own patch for another ticket, just make sure that you're using the current development revision of Django, and **not** an older revision like we are here.
18   
19    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.
20
21.. admonition:: Where to get help:
22
23    If you're having trouble going through this tutorial, please post a message
24    to `django-users`__ or drop by `#django on irc.freenode.net`__ to chat
25    with other Django users who might be able to help.
26
27__ https://code.djangoproject.com/ticket/15315
28__ https://code.djangoproject.com/changeset/16659
29__ http://groups.google.com/group/django-users
30__ irc://irc.freenode.net/django
31
32What you'll need to get started
33===============================
34
35Subversion or Git
36-----------------
37
38For 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.
39
40To 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.
41
42You can download and install Subversion from the `Apache's Subversion Binary Packages page`__.
43
44Alternatively, you can download and install Git from `Git's download page`__.
45
46__ http://subversion.apache.org/packages.html
47__ http://git-scm.com/download
48
49A Trac account
50--------------
51
52Next, you'll need an account for Trac, Django's online issue tracker. Trac allows community members to file tickets for bugs or feature requests, upload patches, discuss changes, and keep track of all the activity on the project.
53
54If you don't have a Trac account yet, visit the `registration page`__ and create one. If you already have an account but have forgotten your password, you can reset it using the `password reset page`__. Having a Trac account will allow you to maintain a consistent identity when contributing to Django and build rapport within the community. In addition, it allows you to claim a ticket for yourself so that other community members know that you're currently working on a patch for it.
55
56__ https://www.djangoproject.com/accounts/register
57__ https://www.djangoproject.com/accounts/password/reset/
58
59A ticket to work on
60-------------------
61
62Once you've setup a Trac account and are logged in, you'll need to find a ticket to work on. Trac has `a very powerful search engine`__ which allows you to filter tickets based on a plethora of criteria. For those of you who've never written a patch for Django before, pay special attention to tickets with the "easy pickings" criterion. These tickets are often much simpler in nature and are great for first time contributors.  Once you're familiar with contributing to Django, you can move on to writing patches for more difficult and complicated tickets.
63
64Finding a ticket
65~~~~~~~~~~~~~~~~
66
67If you just want to get started, try taking a look at the list of `easy tickets that need patches`__ and the `easy tickets that have patches which need improvement`__. If you're familiar with writing tests, you can also look at the list of `easy tickets that need tests`__.
68
69Claiming a ticket
70~~~~~~~~~~~~~~~~~
71
72Once you've found a ticket that looks interesting to you, make sure nobody else has claimed it. To do this, look at the "Assigned to" 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 can either find another bug/feature to work on or contact the developer working on the ticket to offer your help.
73
74To claim a ticket for yourself, click the "accept" radio button under "Action" near the bottom of the page and then click "Submit changes." Keep in mind that 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!
75
76.. admonition:: What do I do if a ticket already has a patch?
77
78    Sometimes a ticket already has a patch, but that patch may still need some adjustments or changes before it can be added to Django. In these cases, the ticket is flagged with the "patch needs improvement" criterion. These tickets can provide a great opportunity to contribute to Django if you don't want to have to write a patch from scratch. Just make sure the ticket is unclaimed before you start your work, and pay particular attention to the suggestions and remarks in the ticket's change history.
79       
80    To work on an existing patch, you'll have to download the existing patch file from the ticket and then apply it to your local copy of Django before making your own modifications.  Detailed steps for this can be found below under `Applying an existing patch to Django`_.
81
82__ https://code.djangoproject.com/query
83__ https://code.djangoproject.com/query?status=new&status=reopened&has_patch=0&easy=1&col=id&col=summary&col=status&col=owner&col=type&col=milestone&order=priority
84__ https://code.djangoproject.com/query?status=new&status=reopened&needs_better_patch=1&easy=1&col=id&col=summary&col=status&col=owner&col=type&col=milestone&order=priority
85__ https://code.djangoproject.com/query?status=new&status=reopened&needs_tests=1&easy=1&col=id&col=summary&col=status&col=owner&col=type&col=milestone&order=priority
86
87How to write your first patch
88=============================
89
90Checking out Django's development version
91-----------------------------------------
92
93The first step to contributing to Django is to check out the Django's current development revision using either Subversion or Git.
94
95.. note::
96
97    For the purposes of this tutorial, we'll be checking out an **older** revision of Django's trunk, specifically revision 16658; however, when working on your own patch for another ticket, make sure you remember to check out the **current** development revision.
98
99From the command line, use the ``cd`` command to navigate to the directory where you'll want your local copy of Django to live.
100
101Using Subversion
102~~~~~~~~~~~~~~~~
103
104If you have Subversion installed, you can run the following command to download revision 16658 of Django:
105
106.. code-block:: bash
107   
108    svn checkout -r 16658 http://code.djangoproject.com/svn/django/trunk/
109
110Using Git
111~~~~~~~~~
112
113Alternatively, 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:
114
115.. code-block:: bash
116
117    git clone https://github.com/django/django.git
118    cd django
119    git checkout 90e8bd48f2
120
121.. note::
122
123    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.
124
125Running Django's test suite
126---------------------------
127
128When 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.
129
130
131Setting Django up to run the test suite
132~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
133
134Before 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:
135
136.. code-block:: python
137   
138    >>> import django
139    >>> django
140    <module 'django' from '/.../django/__init__.pyc'>
141
142If 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`.
143
144If 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.
145
146.. note::
147
148    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>`.
149
150Actually running the Django test suite
151~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
152
153Once Django is setup properly, we can actually run the test suite. Simply ``cd`` into the Django ``tests/`` directory and run:
154
155.. code-block:: bash
156
157    ./runtests.py --settings=test_sqlite
158
159If 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`.
160
161Otherwise, 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.
162
163.. note::
164   
165    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.
166
167Once 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.
168
169More information on how to run Django's test suite can be found in the :doc:`online documentation</internals/contributing/writing-code/unit-tests>`.
170
171Writing a test for your ticket
172------------------------------
173
174In 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.
175
176A 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.
177
178Now for our hands on example.
179
180Writing a test for ticket 15315
181~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
182
183`Ticket 15315`__ describes the following, small feature addition:
184   
185    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.
186
187    The fix is to add a new keyword argument ``widgets=None`` to ``modelform_factory``.
188   
189In 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.
190
191Navigate 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:
192
193.. code-block:: python
194   
195    class FormFieldCallbackTests(TestCase):
196   
197        def test_factory_with_widget_argument(self):
198            """ Regression for #15315: modelform_factory should accept widgets
199                argument
200            """
201            widget = forms.Textarea()
202           
203            # Without the widget, the form field should not have the widget set to a textarea
204            Form = modelform_factory(Person)
205            self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
206           
207            # With a widget, the form field should have the widget set to a textarea
208            Form = modelform_factory(Person, widgets={'name':widget})
209            self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
210       
211        def test_baseform_with_widgets_in_meta(self):
212            ...
213
214This 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.
215
216__ https://code.djangoproject.com/ticket/15315
217
218Running your new test
219~~~~~~~~~~~~~~~~~~~~~
220
221Remember 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:
222
223.. code-block:: bash
224
225    ./runtests.py --settings=test_sqlite model_forms_regress
226
227If 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.
228
229If 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`_.
230
231Making your changes to Django
232-----------------------------
233
234Next we'll actually add the functionality described in `ticket 15315`__ to Django.
235
236Fixing ticket 1315
237~~~~~~~~~~~~~~~~~~
238
239Navigate 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:
240
241.. code-block:: python
242
243    def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
244                          formfield_callback=None,  widgets=None):
245        # Create the inner Meta class. FIXME: ideally, we should be able to
246        # construct a ModelForm without creating and passing in a temporary
247        # inner class.
248     
249        # Build up a list of attributes that the Meta object will have.
250        attrs = {'model': model}
251        if fields is not None:
252            attrs['fields'] = fields
253        if exclude is not None:
254            attrs['exclude'] = exclude
255        if widgets is not None:
256            attrs['widgets'] = widgets
257     
258        # If parent form class already has an inner Meta, the Meta we're
259        # creating needs to inherit from the parent's inner meta.
260        parent = (object,)
261        if hasattr(form, 'Meta'):
262            parent = (form.Meta, object)
263        Meta = type('Meta', parent, attrs)
264     
265        # Give this new form class a reasonable name.
266        class_name = model.__name__ + 'Form'
267     
268        # Class attributes for the new form class.
269        form_class_attrs = {
270            'Meta': Meta,
271            'formfield_callback': formfield_callback
272        }
273     
274        form_metaclass = ModelFormMetaclass
275     
276        if issubclass(form, BaseModelForm) and hasattr(form, '__metaclass__'):
277            form_metaclass = form.__metaclass__
278     
279        return form_metaclass(class_name, (form,), form_class_attrs)
280   
281Notice 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.
282
283Verifying your test now passes
284~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
285
286Once 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:
287
288.. code-block:: bash
289
290    ./runtests.py --settings=test_sqlite model_forms_regress
291
292If 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.
293
294__ https://code.djangoproject.com/ticket/15315
295
296Running Django's test suite (again)
297-----------------------------------
298
299Once 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.
300
301To run the entire Django test suite, ``cd`` into the Django ``tests/`` directory and run:
302
303.. code-block:: bash
304
305    ./runtests.py --settings=test_sqlite
306
307As 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.
308
309Generating a patch file
310-----------------------
311
312Now it's time to actually generate a patch file that can be uploaded to Trac or applied to another copy of Django.
313
314First, 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.
315
316Creating a patch file with Subversion
317~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
318
319If 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:
320
321.. code-block:: bash
322
323    svn diff
324   
325This 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:
326
327.. code-block:: diff
328
329    Index: django/forms/models.py
330    ===================================================================
331    --- django/forms/models.py  (revision 16658)
332    +++ django/forms/models.py  (working copy)
333    @@ -368,7 +368,7 @@
334         __metaclass__ = ModelFormMetaclass
335     
336     def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
337    -                       formfield_callback=None):
338    +                       formfield_callback=None, widgets=None):
339         # Create the inner Meta class. FIXME: ideally, we should be able to
340         # construct a ModelForm without creating and passing in a temporary
341         # inner class.
342    @@ -379,7 +379,9 @@
343             attrs['fields'] = fields
344         if exclude is not None:
345             attrs['exclude'] = exclude
346    -
347    +    if widgets is not None:
348    +        attrs['widgets'] = widgets
349    +   
350         # If parent form class already has an inner Meta, the Meta we're
351         # creating needs to inherit from the parent's inner meta.
352         parent = (object,)
353   
354    Index: tests/regressiontests/model_forms_regress/tests.py
355    ===================================================================
356    --- tests/regressiontests/model_forms_regress/tests.py      (revision 16658)
357    +++ tests/regressiontests/model_forms_regress/tests.py      (working copy)
358    @@ -263,6 +263,20 @@
359     
360     class FormFieldCallbackTests(TestCase):
361     
362    +    def test_factory_with_widget_argument(self):
363    +        """ Regression for #15315: modelform_factory should accept widgets
364    +            argument
365    +        """
366    +        widget = forms.Textarea()
367    +       
368    +        # Without a widget should not set the widget to textarea
369    +        Form = modelform_factory(Person)
370    +        self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
371    +       
372    +        # With a widget should not set the widget to textarea
373    +        Form = modelform_factory(Person, widgets={'name':widget})
374    +        self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
375    +
376         def test_baseform_with_widgets_in_meta(self):
377             """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors."""
378             widget = forms.Textarea()
379
380If the patch's content looks okay, you can run the following command to save the patch file to your current working directory.
381
382.. code-block:: bash
383
384    svn diff > myfancynewpatch.diff
385
386You should now have a file in the root Django directory called ``myfancynewpatch.diff``. This patch file that contains all your changes.
387
388Creating a patch file with Git
389~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
390
391For 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:
392
393.. code-block:: bash
394
395    git diff
396   
397This 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:
398   
399.. code-block:: diff
400
401    diff --git a/django/forms/models.py b/django/forms/models.py
402    index 527da5e..a93f21c 100644
403    --- a/django/forms/models.py
404    +++ b/django/forms/models.py
405    @@ -368,7 +368,7 @@ class ModelForm(BaseModelForm):
406         __metaclass__ = ModelFormMetaclass
407     
408     def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
409    -                       formfield_callback=None):
410    +                       formfield_callback=None, widgets=None):
411         # Create the inner Meta class. FIXME: ideally, we should be able to
412         # construct a ModelForm without creating and passing in a temporary
413         # inner class.
414    @@ -379,6 +379,8 @@ def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
415             attrs['fields'] = fields
416         if exclude is not None:
417             attrs['exclude'] = exclude
418    +    if widgets is not None:
419    +        attrs['widgets'] = widgets
420     
421         # If parent form class already has an inner Meta, the Meta we're
422         # creating needs to inherit from the parent's inner meta.
423   
424    diff --git a/tests/regressiontests/model_forms_regress/tests.py b/tests/regressiontests/model_forms_regress/tests
425    index 9860c2e..d2d9aa3 100644
426    --- a/tests/regressiontests/model_forms_regress/tests.py
427    +++ b/tests/regressiontests/model_forms_regress/tests.py
428    @@ -263,6 +263,20 @@ class URLFieldTests(TestCase):
429     
430     class FormFieldCallbackTests(TestCase):
431     
432    +    def test_factory_with_widget_argument(self):
433    +        """ Regression for #15315: modelform_factory should accept widgets
434    +            argument
435    +        """
436    +        widget = forms.Textarea()
437    +       
438    +        # Without a widget should not set the widget to textarea
439    +        Form = modelform_factory(Person)
440    +        self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
441    +       
442    +        # With a widget should not set the widget to textarea
443    +        Form = modelform_factory(Person, widgets={'name':widget})
444    +        self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea)
445    +
446         def test_baseform_with_widgets_in_meta(self):
447             """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors."""
448             widget = forms.Textarea()
449
450The 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.
451
452.. code-block:: bash
453
454    git diff > myfancynewpatch.diff
455
456You should now have a file in the root Django directory called ``myfancynewpatch.diff``. This patch file that contains all your changes.
457
458Working together with the community
459===================================
460
461Now that you've generated your first patch, you can go on and find a ticket of your own to write a patch for. Before you do that, let's go over a few more useful points for contributing to Django.
462
463Submitting a patch to Trac
464--------------------------
465
466Placeholder...
467
468Applying an existing patch to Django
469------------------------------------
470
471Placeholder...
472
473
474.. note::
475   
476    Some of this text comes from Django's documentation on :doc:`submitting patches</internals/contributing/writing-code/submitting-patches>`
477
478# Alternately, have a conclusion section which contains summary and how to submit a patch to django, and then move "applying an existing patch" to an early section.
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499