Code quality / patch quality

Slides for a talk I've given over the years (first in 2008, many times since). Talking about good practices contributing to Django and other large open source projects.

Transcript

1. Code quality Patch quality Malcolm Tredinnick malcolm.tredinnick@gmail.com
2. Code Quality Matters
3. “Have you ever written a library before? It's like people seeing you in your underwear. You gotta make sure it's clean.”
4. Do The Basics Properly
5. Yr Doin' It Wrng! • If the word “print” is still in your patch – Similarly for the phrase “import pdb”
6. Yr Doin' It Wrng! • If you don't have a test or a really good reason – Your reason is never going to be good enough. – Fail before; pass afterwards
7. Yr Doin' It Wrng! • If you think “PEP 8” is the name of a new energy drink
8. How Hard Can It Be(tm)? • They're called patches for a reason. • Go to the top of the project tree. • Then: svn diff (or git diff or ...) – “svn add” works locally • Even if you're a translator or a documenter.
9. Code style is somewhat opinionated
10. Your opinions don't matter (that much)
11. Read the contributing document
12. Code is... • Written once • Read many, many times
13. Comments should last
14. Comments should be correct!
15. Fix problems not symptoms!
16. Symptom fixing...
17. The template code below will cause an infinite memory-eating loop if context_processors.auth is enabled. My main issue with that is that it was incredibly hard to debug for me, when I passed my own 'perms' queryset to a template. ... {% for perm in perms %} {% endfor %} Ticket #8182: Infinite loop iterating over PermWrapper Infinite loop iterating over PermWrapper
18. django/contrib/auth/models.py | 22 ++++++++++++++++++++++ django/core/context_processors.py | 5 +++++ docs/authentication.txt | 11 ++++++++--- 3 files changed, 35 insertions(+), 3 deletions(-)
20. Where else does it happen?
21. Warning: may be hard!
22. Author: mtredinnick Date: Thu Jun 26 11:01:21 2008 +1000 ... This is the same problem as [7597] but for values() field specifications, so this covers the second case where Django adds extra stuff to the select-clause. r7740
23. Author: mtredinnick Date: Mon Jul 28 04:16:17 2008 +1000 ... Fixed a missed case of promoting table joins when using disjunctive filters. r8107
24. r8783 Author: mtredinnick Date: Mon Sep 1 18:43:55 2008 -0700 ... Fixed an oversight when I first fixed ordering on nullable foreign keys.
25. r8853 Author: mtredinnick Date: Tue Sep 2 06:52:07 2008 -0700 .... + def promote_alias_chain(self, chain,...): + """ + Walks along a chain of aliases, promoting the first + nullable join and any joins following that. If + 'must_promote' is True, all the aliases in + the chain are promoted. + """ + for alias in chain: + if self.promote_alias(alias, must_promote): + must_promote = True +
26. Know your limitations
27. The crowd is smarter than you! (the code mostly works)
28. “Here is a trivial patch”
29. Translation: “here are some bugs I thought you should have”
30. Research is not a four letter word
31. • Django runs out-of-the-box on databases. Plural. – SQLite, MySQL, PostgreSQL, Oracle • External backends, too • Separate solution approach into pieces – It has a chance of working and here's a good start. – It has a perfect implementation.
32. • Django runs on multiple operating systems – Windows, Linux, Mac, ... • Some things are hard – File handling, locking, ... – Character encoding • Best defense is “don't do that”. • Next best is a good offense (Research!)
33. • There are RFCs. – They apply in this universe. • There are competing specs – Real world software beats paper • There are contradictory requirements – Duh!
34. Python needs research, too • Pickle has mulitple protocols: – 0 and 2 are important • __len__() is eeevviiilll – With good reason – Swallows exceptions • TypeError • Others (Python 2.3, Python 2.6.0 – 2.6.1) • Unicode matters. So does UTF-8
35. Always contribute! Best beats perfect