desktopcouch

Merge lp://qastaging/~jderose/desktopcouch/ctx-should-default-to-None into lp://qastaging/desktopcouch

ctx-should-default-to-None
Merge into trunk

Proposed by Jason Gerard DeRose on 2010-11-08

Status:

Merged

Approved by:

Vincenzo Di Somma on 2010-11-16

Approved revision:

200

Merged at revision:

211

Proposed branch:

lp://qastaging/~jderose/desktopcouch/ctx-should-default-to-None

Merge into:

lp://qastaging/desktopcouch

Diff against target:

14 lines (+3/-1)

1 file modified

desktopcouch/records/server.py (+3/-1)

To merge this branch:

bzr merge lp://qastaging/~jderose/desktopcouch/ctx-should-default-to-None

Undecided

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Vincenzo Di Somma (community)		Approve on 2010-11-16
Eric Casteleijn (community)	2010-11-08	Approve on 2010-11-15
Review via email: mp+40313@code.qastaging.launchpad.net

Commit message

Make context function-argument default-value None so that callers need not concern themselves with complex imports.

Description of the change

Small change to make it easier for 3rd-party apps to use destopcouch test idioms (this is a problem for dmedia).

Revision history for this message

Eric Casteleijn (thisfred) wrote on 2010-11-12:

I think I'm missing the point of this change. In what scenario would this make a difference? Is it to do with the import time instantiation of default arguments?

Revision history for this message

Jason Gerard DeRose (jderose) wrote on 2010-11-13:

It's so that internal details (that ctx defaults to desktopcouch.local_files.DEFAULT_CONTEXT) don't leak into subclasses, other functions/classes using it. It's also best-practice in the Python standard library... optional/keyword args default to simple, predictable values, most often None, sometime True/False, etc.

For example, in dmedia I need to provide a custom context when testing (Chad help me with this so I can test the same way desktopcouch itself does), but I don't want to deal with the desktopcouch.local_files.DEFAULT_CONTEXT detail myself.

See the dmedia MetaStore.__init__() on line 109: http://bazaar.launchpad.net/~jderose/dmedia/trunk/annotate/head%3A/dmedialib/metastore.py?start_revid=104