diff --git a/docs/index.txt b/docs/index.txt index a6d9ed2..e6eb77c 100644 --- a/docs/index.txt +++ b/docs/index.txt @@ -47,7 +47,8 @@ Are you new to Django or to programming? This is the place to start! :doc:`Part 4 ` * **Advanced Tutorials:** - :doc:`How to write reusable apps ` + :doc:`How to write reusable apps ` | + :doc:`Writing your first patch for Django ` The model layer =============== diff --git a/docs/intro/contributing.txt b/docs/intro/contributing.txt new file mode 100644 index 0000000..c365a39 --- /dev/null +++ b/docs/intro/contributing.txt @@ -0,0 +1,549 @@ +=================================== +Writing your first patch for Django +=================================== + +Introduction +============ + +Interested in 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. + +Contributing back to Django itself is the best way to see your own concerns +addressed. This may seem daunting at first, but it's really pretty simple. +We'll walk you through the entire process, so you can learn by example. + +Who's this tutorial for? +------------------------ + +For this tutorial, we expect that you have at least a basic understanding of +how Django works. This means you should be comfortable going through the +existing tutorials on :doc:`writing your first Django app`. +In addition, you should have a good understanding of Python itself. But if you +don't, `Dive Into Python`__ is a fantastic (and free) online book for beginning +Python programmers. + +Those of you who are unfamiliar with version control systems and Trac will find +that this tutorial and its links include just enough information to get started. +However, you'll probably want to read some more about these different tools if +you plan on contributing to Django regularly. + +For the most part though, this tutorial tries to explain as much as possible, +so that it can be of use to the widest audience. + +.. admonition:: Where to get help: + + If you're having trouble going through this tutorial, please post a message + to `django-users`__ or drop by `#django on irc.freenode.net`__ to chat + with other Django users who might be able to help. + +__ http://diveintopython.net/toc/index.html +__ http://groups.google.com/group/django-users +__ irc://irc.freenode.net/django + +What does this tutorial cover? +------------------------------ + +We'll be walking you through contributing a patch to Django for the first time. +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. +* Running Django's test suite. +* Writing a test for your patch. +* Writing the code for your patch. +* Testing your patch. +* Generating a patch file for your changes. +* Where to look for more information. + +Once you're done with the tutorial, you can look through the rest of +:doc:`Django's documentation on contributing`. +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. + +Installing Git +============== + +For this tutorial, you'll need Git installed to download the current +development version of Django and to generate patch files for the changes you +make. + +To check whether or not you have Git installed, enter ``git`` into the command +line. If you get messages saying that this command could be found, you'll have +to download and install it, see `Git's download page`__. + +.. note:: + + If you're not that familiar with Git, you can always find out more about + its commands (once it's installed) by typing ``git help`` into the command + line. + +__ http://git-scm.com/download + +Getting a copy of Django's development version +============================================== + +The first step to contributing to Django is to get a copy of the source code. +If you're using ``virtualenv``, it's better if you don't use ``pip`` to install +Django, since that can cause Django's setup.py script to be run. Follow the +instructions below to use Git to checkout a copy of Django manually. + +From the command line, use the ``cd`` command to navigate to the directory +where you'll want your local copy of Django to live. + +Download the Django source code repository using the following command:: + + git clone https://github.com/django/django.git + +Rolling back to a previous revision of Django +============================================= + +For this tutorial, we'll be using `ticket #15315`__ as a case study, so we'll +be using an older revision of Django from 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. + +**Keep in mind that while we'll be using an older revision of Django's trunk +for the purposes of the tutorial below, you should always use the current +development revision of Django when working on your own patch for a ticket!** + +.. note:: + + 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 + using the revision of Django just prior to that, revision 16658 (this is + from when Django used Subversion rather than Git for source control). + +__ https://code.djangoproject.com/ticket/15315 +__ https://code.djangoproject.com/changeset/16659 + +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:: + + git checkout 1f770c62da69866835bb8fc9355128b6b493a97e + +Running Django's test suite for the first time +============================================== + +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 stills +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. + + +Setting Django up to run the test suite +--------------------------------------- + +Before 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:: + + >>> import django + >>> django + + +If 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``. + +If 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. + +.. note:: + + 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. Using + ``virtualenv`` with the ``--no-site-packages`` option isolates your local + copy of Django from the rest of your system and avoids potential conflicts. + + Make sure you don't use ``pip`` to install Django, since that causes + Django's setup.py script to be run. If you do, you'll lose Django's root + directory and end up with only the code installed. So you'll still need to + use Git to check out a copy of Django, and then add it to your + ``PYTHONPATH``. + +Running the full test suite +--------------------------- + +Once Django is setup properly, we can actually run the test suite. Simply +``cd`` into the Django ``tests/`` directory and run: + +.. code-block:: bash + + ./runtests.py --settings=test_sqlite + +If you get an ``ImportError: No module named django.contrib`` error, you still +need to add your current copy of Django to your ``PYTHONPATH``. + +Otherwise, 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. + +.. note:: + + 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 passing tests. + +Once 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, however, the latest +Django trunk may not always be stable. In this case you might run into a couple +of failures that were later fixed: + +* ``test_week_view_allow_future + (regressiontests.generic_views.dates.WeekArchiveViewTests)``: This test + happened to pass in 2011 when it was added because January 1, 2012 is a + Sunday. It was fixed to pass regardless of the year in which it was run + in `[b9910bdd]`__. + +* ``int() argument must be a string or a number, not 'list'`` and + ``"ManagementForm data is missing or has been tampered with`` errors in the + ``contrib.formtools`` tests: These tests sometimes failed if you had a buggy + version of ``simplejson`` (2.0.9 in particular). This was fixed in + `[358e5a80]`__. + +__ https://github.com/django/django/commit/b9910bdd/ +__ https://github.com/django/django/commit/358e5a80/ + +If you get failures or errors besides what's described above, make sure you've +followed all of the previous steps properly. See :ref:`running-unit-tests` for +more information. + +Writing a test for your ticket +============================== + +In 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. + +A good way to do this is to write your new tests first, before making any +changes to the code. This style of development is called +`test-driven development`__ and can be applied to both entire projects and +single patches. After writing your tests, you then run them to make sure that +they do indeed fail (since you haven't fixed that bug or added that feature +yet). 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 at preventing that bug from reoccurring down the road. + +Now for our hands on example. + +__ http://en.wikipedia.org/wiki/Test-driven_development + +Writing a test for ticket #15315 +-------------------------------- + +`Ticket #15315`__ describes the following, small feature addition: + + Since Django 1.2, the ``ModelForm`` class can override widgets by + specifying a ``'widgets'`` attribute in ``Meta``, similar to ``'fields'`` + or ``'exclude'``. The ``modelform_factory`` function doesn't have any way + to specify widgets directly, so the only option is to define a new + ``ModelForm`` subclass with a ``'widget'`` attribute in ``Meta``, and then + pass that class to ``modelform_factory`` using the ``'form'`` argument. + This is more complex than it needs to be. + + The fix is to add a new keyword argument ``widgets=None`` to + ``modelform_factory``. + +In 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 functions +correctly and continues to function correctly in the future. + +Navigate to 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:: + + class FormFieldCallbackTests(TestCase): + + def test_factory_with_widget_argument(self): + """ Regression for #15315: modelform_factory should accept widgets + argument + """ + widget = forms.Textarea() + + # Without the widget, the form field should not have the widget set to a textarea + Form = modelform_factory(Person) + self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea) + + # With a widget, the form field should have the widget set to a textarea + Form = modelform_factory(Person, widgets={'name':widget}) + self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea) + + def test_baseform_with_widgets_in_meta(self): + ... + +This 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 are using the specified widgets. Now we have the +test for our patch written. + +.. admonition:: But this testing thing looks kinda hard... + + If you've never had to deal with tests before, they can look a little hard + to write at first glance. Fortunately, testing is a *very* big subject in + computer programming, so there's lots of information out there: + + * A good first look at writing tests for Django can be found in the + documentation on :doc:`Testing Django applications`. + * Dive Into Python (a free online book for beginning Python developers) + includes a great `introduction to Unit Testing`__. + * After reading those, if you want something a little meatier to sink + your teeth into, there's always the `Python unittest documentation`__. + +__ https://code.djangoproject.com/ticket/15315 +__ http://diveintopython.net/unit_testing/index.html +__ http://docs.python.org/library/unittest.html + +Running your new test +--------------------- + +Remember 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: + +.. code-block:: bash + + ./runtests.py --settings=test_sqlite model_forms_regress + +If 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. + +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. It's also possible +that you have a second copy of Django on your ``PYTHONPATH`` that is taking +priority over this copy, and therefore it may 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`_. + +Writing the code for your ticket +================================ + +Next we'll be adding the functionality described in `ticket #15315`__ to Django. + +Writing the code for ticket #15315 +---------------------------------- + +Navigate to the ``django/django/forms/`` folder and open the ``models.py`` +file. Find the ``modelform_factory`` function on line 370 and modify **the +first part of it** so that it reads as follows: + +.. code-block:: python + + def modelform_factory(model, form=ModelForm, fields=None, exclude=None, + formfield_callback=None, widgets=None): + # Create the inner Meta class. FIXME: ideally, we should be able to + # construct a ModelForm without creating and passing in a temporary + # inner class. + + # Build up a list of attributes that the Meta object will have. + attrs = {'model': model} + if fields is not None: + attrs['fields'] = fields + if exclude is not None: + attrs['exclude'] = exclude + if widgets is not None: + attrs['widgets'] = widgets + + # If parent form class already has an inner Meta, the Meta we're + # creating needs to inherit from the parent's inner meta. + ... + +Notice that we're changing one line of code to add a new ``widgets`` keyword +argument to the function's signature, and then we're adding two lines of code +a little below that which set ``attrs['widgets']`` to the value of our new +keyword argument if it was supplied. + +Verifying your test now passes +------------------------------ + +Once you're done modifying ``modelform_factory``, we need to make sure that the +test we wrote earlier passes now, so we can see whether the code we wrote above +is working right. To run the tests in the ``model_forms_regress`` folder, +``cd`` into the Django ``tests/`` directory and run: + +.. code-block:: bash + + ./runtests.py --settings=test_sqlite model_forms_regress + +If 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. + +__ https://code.djangoproject.com/ticket/15315 + +Running Django's test suite for the second time +=============================================== + +Once you've verified that your patch and your test are working correctly, it's +a good idea to run the entire Django test suite just to verify that your change +hasn't introduced any bugs into 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 that might otherwise go unnoticed. + +To run the entire Django test suite, ``cd`` into the Django ``tests/`` +directory and run: + +.. code-block:: bash + + ./runtests.py --settings=test_sqlite + +As 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. + +Generating a patch for your changes +=================================== + +Now it's time to generate a patch file that can be uploaded to Trac or applied +to another copy of Django. To get a look at the content of your patch, run the +following command:: + + git diff + +This 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. + +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:: + + git diff > 15315.diff + +You should now have a file in the root Django directory called ``15315.diff``. +This patch file contains all your changes and should look this: + +.. code-block:: diff + + Index: django/forms/models.py + =================================================================== + --- django/forms/models.py (revision 16658) + +++ django/forms/models.py (working copy) + @@ -368,7 +368,7 @@ + __metaclass__ = ModelFormMetaclass + + def modelform_factory(model, form=ModelForm, fields=None, exclude=None, + - formfield_callback=None): + + formfield_callback=None, widgets=None): + # Create the inner Meta class. FIXME: ideally, we should be able to + # construct a ModelForm without creating and passing in a temporary + # inner class. + @@ -379,7 +379,9 @@ + attrs['fields'] = fields + if exclude is not None: + attrs['exclude'] = exclude + - + + if widgets is not None: + + attrs['widgets'] = widgets + + + # If parent form class already has an inner Meta, the Meta we're + # creating needs to inherit from the parent's inner meta. + parent = (object,) + + Index: tests/regressiontests/model_forms_regress/tests.py + =================================================================== + --- tests/regressiontests/model_forms_regress/tests.py (revision 16658) + +++ tests/regressiontests/model_forms_regress/tests.py (working copy) + @@ -263,6 +263,20 @@ + + class FormFieldCallbackTests(TestCase): + + + def test_factory_with_widget_argument(self): + + """ Regression for #15315: modelform_factory should accept widgets + + argument + + """ + + widget = forms.Textarea() + + + + # Without a widget should not set the widget to textarea + + Form = modelform_factory(Person) + + self.assertNotEqual(Form.base_fields['name'].widget.__class__, forms.Textarea) + + + + # With a widget should not set the widget to textarea + + Form = modelform_factory(Person, widgets={'name':widget}) + + self.assertEqual(Form.base_fields['name'].widget.__class__, forms.Textarea) + + + def test_baseform_with_widgets_in_meta(self): + """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors.""" + widget = forms.Textarea() + +So what do I do next? +===================== + +Congratulations, you've generated your very first Django patch! Now that you've +got that under your belt, you can put those skills to good use by helping to +improve Django's codebase. Generating patches and attaching them to Trac +tickets is useful, however, you can also :doc:`submit pull requests on Github +`. + +More information for new contributors +------------------------------------- + +Before you get too into writing patches for Django, there's a little more +information on contributing that you should probably take a look at: + +* You should make sure to read Django's documentation on + :doc:`claiming tickets and submitting patches + `. + It covers Trac etiquette, how to claim tickets for yourself, expected + coding style for patches, and many other important details. +* First time contributors should also read Django's :doc:`documentation + for first time contributors`. + It has lots of good advice for those of us who are new to helping out + with Django. +* After those, if you're still hungry for more information about + contributing, you can always browse through the rest of + :doc:`Django's documentation on contributing`. + It contains a ton of useful information and should be your first source + for answering any questions you might have. + +Finding your first real ticket +------------------------------ + +Once you've looked through some of that information, you'll be ready to go out +and find a ticket of your own to write a patch for. 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. + +If you just want to get started already (and nobody would blame you!), 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`__. Just remember to follow the guidelines about +claiming tickets that were mentioned in the link to Django's documentation on +:doc:`claiming tickets and submitting patches +`. + +__ 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 +__ 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 +__ 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 diff --git a/docs/intro/index.txt b/docs/intro/index.txt index afb1825..bca2d77 100644 --- a/docs/intro/index.txt +++ b/docs/intro/index.txt @@ -15,6 +15,7 @@ place: read this material to quickly get up and running. tutorial04 reusable-apps whatsnext + contributing .. seealso::