Add a tutorial for first time Django contributors

[Taavi Taijala]

There's been some discussion about adding a tutorial for first time Django contributors who may not be familiar with the processes and tools involved in submitting patches to Django. I've written a rough draft of a tutorial for this.

You can view a rendered copy of the RST at ​http://www.taijala.com/tutorial.html

Some of the links may not work though, since I only uploaded the tutorial document.

[Julien Phalip]

Awesome work, thanks so much for doing this!

As discussed during the sprint, the main thing is to avoid duplication of text between this new step-by-step tutorial and the existing contributing docs, especially: ​https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/

[Taavi Taijala]

Removed Trac instructions in favor of linking to that documentation

[edunham]

taavi,

It's looking good! Some thoughts on the current version of the document (I accessed it on ​http://www.taijala.com/tutorial.html):

• Be explicit about the level of experience (with Python, Django, ticket trackers, version control...) you expect from the audience -- maybe even in the first pargraph. "This tutorial assumes that you've been through the introductory Django guide at ​https://docs.djangoproject.com/en/1.3/intro/tutorial01/ and have some experience with Python [link to tutorial at appropriate level]"

• For the link to "a very powerful search engine" in Trac, it might be useful to also link an explanation of what the search's fields mean (if you aren't assuming familiarity with ticket trackers). https://code.djangoproject.com/wiki/TracTickets#TicketFields would work ok, but there are probably better tutorials out there.

• In the note starting with "if you're a savvy djangonaut...", place more emphasis on the "don't install Django with Pip" advice. It would be easy to overlook in its current format.

• The heading "Actually running the test suite" would be clearer without the word "actually". The second half of the article appears to actually have a higher than usual incidence of the word "actually". It might actually be easier to read if you removed most of the instances or replaced them with synonyms. Then again, it's actually just my personal opinion that "actually" can usually be removed without changing a sentence's meaning and is distracting if you use it too much. :)

• The "Writing a Test for your Ticket" section does a good job of covering the basics of what one needs to know, but would be even better if it included some links to external resources on how to design tests, testing philosophies, or other information.

• Formatting the text of the example differently would make it clearer which instructions are for any patch and which are for the example ticket specifically. Perhaps an indent or a change of background color?

• Finally, a table of contents at the top to outline the various sections will make the organization of the article significantly easier to follow.

Over all, this is looking like an excellent start to the article! Thank you for putting it together -- it'll be really beneficial to newbies like me.

-edunham

[Taavi Taijala]

Fixed some incorrect links

[Taavi Taijala]

I've updated the tutorial to address pretty much all of the feedback I've received so far. In addition, the rendered version at ​http://www.taijala.com/tutorial.html has been updated to match.

If anyone else has any feedback on the tutorial, please do post a comment.

[Gabriel Hurley]

This is looking really good. Tremendous effort.

My big comment is that I think the transition from generic initial setup material blurs into the tutorial portion in a way that may confuse people during the "Getting a copy of Django’s development version" section. As it stands currently it's easy to think (from the eye jumping from the title to the code snippet) that you check out Django's development version by fetching revision 16558. I would move the steps for checking out a specific revision of Django out of there and into a separate section afterward which decisively kicks off the "tutorial" portion.

One small typo on line 70: "The first step to contributing to Django is to check out the Django’s current development"

Same on line 185: "Navigate to the Django's tests/regressiontests/model_forms_regress/ folder and open the tests.py file."

Lastly, this obviously all needs to be wrapped to an 80-character margin once the text is final.

Thanks so much for taking this on. Great work!

[Taavi Taijala]

Gabriel,

Thanks for the excellent feedback. I've split that section up into two sections "Getting a copy of Django's development version" and "Rolling back to a previous revision of Django." I think this makes the whole thing much clearer. That section was somewhat confusing and crowded before, so this really helps. Let me know what you think.

If anyone else has any feedback, please do let me know. The feedback I've received so far has greatly improved the tutorial. Once everything looks good, I'll convert the tutorial to 80-character lines.

I've added the updated patch below, and updated the rendered version on my website.

P.S. Does anyone have any idea where this tutorial should go? Russel mentioned in passing at DjangoCon that something like this might fight at the end of the introductory tutorial series. Regardless of where it goes though, I would like it to have high visibility so that we can encourage as many new community members to contribute as possible.

[Wim Feijen <wim@…>]

Hi taavi, nice!

Because you asked for feedback:

1. myfancynewpatch.diff is a good name; but maybe patch_for_ticket_xxxxx is better? You might even set a convention here. :)
2. What I do when I run tests, I just make a symlink to django in the test directory, so I don't need to mess with python paths.

Great great work, I wish I had read it two years before! :)

[Julien Phalip]

Replying to Wim Feijen <wim@…>:

1. myfancynewpatch.diff is a good name; but maybe patch_for_ticket_xxxxx is better? You might even set a convention here. :)

Everyone is different, but <ticket number>.<short description of what the patch is about>.diff (e.g. 15315.modelform-factory-widget-support.diff) is a good convention, I think.

2. What I do when I run tests, I just make a symlink to django in the test directory, so I don't need to mess with python paths.

I wouldn't recommend doing this in the tutorial. Having symlinks hanging around is not very clean ;-) The PYTHONPATH approach is correct here.

[Gabriel Hurley]

Ditto to what Julien said about the patch naming convention. Leading off with the ticket number is a good practice.

Symlinks are also bad because certain operating systems don't support them. Let's do our part and inform people about PYTHONPATH.

[Preston Holmes]

Minor point but pip install -e path/to/trunk will get a link to django installed in a virtualenv correctly for dev work, not sure it's needed in these docs though. Bravo for doing this.

[Bas Peschier]

Oh wow, really nice work!

One a ​Glyph Lefkowitz-style note, but why not include the python-powered Mercurial? O:)

[Tim Graham]

Hi Taavi, Nice work on this. I've updated the patch a bit (removing SVN instructions since they're no longer necessary). One thing I noticed is that it looks like the tests don't pass entirely at this particular revision. I get 1 failure and 5 errors in django/contrib/formtools/tests/wizard/ and django/tests/regressiontests/generic_views/dates.py. I'm thinking we could add a note about this ("trunk may not be stable on all platforms"), rather than completely rewriting the tutorial to find some example where all the tests do pass.

[Tim Graham]

Figured out the deal with the test failures and updated the patch.

For review: ​http://techytim.com/django/16779/intro/contributing.html

[Russell Keith-Magee]

On the whole -- this is a great start. A few comments:

• Given the subject matter, it's probably better to direct people at the django-dev mailing list/IRC rooms. If someone is willing to help contribute and they want to know how to do so, -dev is a better match.

• Using a sample ticket is a good idea, but it's potentially confusing because it was committed in the SVN era; Git doesn't have "revision 16658". In the interests of clarity, it might be better to pick on a bug that was fixed some time this year so we're only dealing with git hashes. It's also a bad candidate because the suite didn't pass cleanly before; while it's certainly worth pointing out that trunk may not necessarily pass all tests, a perfect exemplar shouldn't have that problem.

• The admonition about installing via pip is a bit weird. Firstly, you can certainly use pip to install into a virtualenv; you just need to make sure pip is in your virtualenv (and if you've got virtualenv 1.4+, pip is there by default). Secondly, you can install with pip -e, which does a full git checkout. That means you can build a virtualenv that has a live, editable source checkout. I'm not entirely sure what the right response is here -- mentioning virtualenv is a good thing, but we don't want to spend a page of this tutorial teaching people how to use it. There's a middle ground we need to find.

• As a 'show by doing' example, it might be worth doing the 'fix' in two parts; e.g., first attempt, just add the widgets=None argument, then run the test suite to show that the error received is different, then write the rest of the fix, and run the suite to show that the test now passes. Essentially, trying to show the idea *why* we write the tests by showing one failing during development.

• There's no mention (even as a sidebar) about testing against different databases. test_sqlite settings are there for a reason, but a tutorial on development should at least point out that it's possible (and sometimes necessary) to run tests against a different database.

• There's no mention of documentation requirements. This is a new feature, so it *should* have a documentation component, modifying the docs for modelform_factory to describe the new argument. I notice that I didn't write a docs addition for that patch... so I'm a very bad boy in this case.

• There's no mention of uploading ticket metadata when you upload your ticket. Once you've uploaded a ticket, you should be flicking the flags on the ticket to say "has patch", "doesn't need tests", etc, so others can find it for review.

• On the subject of which, "what's next" is "someone else reviews your ticket"... which is something this tutorial doesn't mention. Contributing doesn't always mean writing a patch from scratch. Making sure other patches meet the base criteria is also a very helpful contribution. Find someone else's patch, and make sure they've followed this tutorial -- and mark the ticket as ready for checkin. Might as well take the opportunity to teach people how to get all the way to the end.

[Łukasz Rekucki]

Here's my 2 cents:

• Things like ./runtests.py won't work on Windows unless someone has the ​Windows Launcher. python runtests.py works fine.

• There is a lot of talk about PYTHONPATH, but no real mention of how to check or alter it. If we assume, the person reading knows how PYTHONPATH works, then this whole paragraph could be a lot shorter. If not, then it should show how to set it up properly.

• Sadly, we have a few testcases which depends on PIL. And on Windows, installing PIL into a virtualenv in next to impossible (at least in my expirience), so I actually never use --no-site-packages there.

• Speaking of which, there is no mentioned of what the test suite output really means. Most people will get a lot of skipped tests unless they install extra libraries. It would be good to mentioned that, as they might as well be skipping tests related to what he they're changing.

• I know we don't want to teach people Git in that tutorial, but it would be better to ask people to commit their changes and then generate a patch using git show or even better git format-patch. git diff invoked like this will omit any files a person may have added. Or maybe we can leave this for a "advanced" part.

• The result of git diff needs updating ;)

[Tim Graham]

All good feedback, thanks Russ and Lukasz.

I think revising the tutorial to use a more recent change in the git era is a reasonable request.

Aymeric has suggested ​https://github.com/django/django/commit/7cc4068c4470876c526830778cbdac2fdfd6dc26 as an example of a ticket where the triage workflow was applied correctly: "there was a good discussion between community members, and then between core developers, and finally a commit."

My feeling would be to go with something a bit simpler since this tutorial is just "Writing your first patch", e.g. ​https://github.com/django/django/commit/ac2052ebc84c45709ab5f0f25e685bf656ce79bc as it seems like something a newbie could relate to and touches just 4 files, 1 code, 1 css, 1 test, 1 doc (doesn't get much simpler).

I'm open to other suggestions.

[Tim Graham]

Incorporated feedback from Russ and Lukasz

[Tim Graham]

I've updated the patch as well as ​​http://techytim.com/django/16779/intro/contributing.html based on the feedback above.

[Claude Paroz]

Thanks, it looks very nice!

Found 2 typos:

• that Django stills works -> still
• hands on example -> hands-on

About the command to run the test suite, what about always prefixing the command with PYTHONPATH=.., then we could drop the convoluted paragraph about setting the path.

I wonder if we should warn in "What's next" that it might take time (weeks or months) before a patch gets reviewed depending on the severity of the issue and available time of Django developers.

[Tim Graham]

fixed typos

[Tim Graham]

Thanks for the review, Claude. I've fixed the typos in the latest patch.

I wouldn't be totally opposed to the PYTHONPATH prefix as it seems it would make the tutorial more newbie friendly. On the other hand, it seems fairly annoying to have to type that in each time, particularly if it's a long path. I suppose we could assume people would just use the history in their terminal...

The FAQ for new contributors which is linked from the tutorial does address the weeks/months issue. I'm not sure if we should duplicate that here.

[Claude Paroz]

Replying to timo:

I wouldn't be totally opposed to the PYTHONPATH prefix as it seems it would make the tutorial more newbie friendly. On the other hand, it seems fairly annoying to have to type that in each time, particularly if it's a long path. I suppose we could assume people would just use the history in their terminal...

Well, I really thought PYTHONPATH=.., not any other path, based on the fact that we are inside the tests directory, so with .. we are sure that the current django tree is used.

The FAQ for new contributors which is linked from the tutorial does address the weeks/months issue. I'm not sure if we should duplicate that here.

OK, fine.

[Tim Graham]

I've made the following updates based on feedback from the django-developers thread:

• Recommended running tests with PYTHONPATH=.. since both Aymeric and Claude suggested that.
• Removed the PYTHONPATH section since it's obsolete given the above
• Recommended Git Bash, per Aymeric's suggestion. I don't have any experience with Windows to elaborate here, so if you have input, please chime in with suggested text.
• Added a recommendation for virtualenv and eliminated the "advanced" in "For advanced users who wish to use virtualenv".

[Preston Holmes]

OK - sorry or the late feedback - but better late than never.

I don't think we want to encourage pip -e github-url as a pattern - this puts the git checkout inside the virtualenv

virtualenv's are something that should be possible to blow away - and having a repo where you are doing development work in there isn't a good pattern to demonstrate

Instead following the line to git clone, the virtualenv pip instruction should be:

pip -e /path/to/your/local/clone

This will create the live link in the virtualenv, but keep the dev repo wherever you want on disk.

---

Suggested rewording:

151 +be using an older revision of Django from before that ticket's patch was
152 +applied.

so we'll rewind Django's version history in git to before that ticket's patch was applied

Helps clarify that all older revisions are in the one repo.

---

I'm reluctantly OK with showing the diff + trac attachment approach, but I'd like to advocate for the git workflow just a bit more strongly. I also think we need to help people get their upstaged changes cleared out for future work.

replace:

however, you can also :doc:`submit pull requests on Github
558 +</internals/contributing/writing-code/working-with-git>`.

with:

however, since we are using git - adopting a more git oriented workflow </internals/contributing/writing-code/working-with-git> is recommended. To get your git branch back to a good starting point, we can do the following since we never committed our changes locally:

git reset --hard head
git checkout master

[Tim Graham]

Updated for Preston's comments

[Tim Graham <timograham@…>]

In 7058b595b668b49daef8beb76a2b5c5f1d991b00:

Fixed #16779 - Added a contributing tutorial

Thank-you Taavi Taijala for the draft patch!

[Tim Graham <timograham@…>]

In 04adb7626ba6e578d172df292e86ae384d195939:

[1.5.X] Fixed #16779 - Added a contributing tutorial

Thank-you Taavi Taijala for the draft patch!

Backport of 7058b595b6 from master