80
80
Bazaar supports Python from 2.4 through 2.6, and in the future we want to
81
support Python 3.0. Avoid using language features added in 2.5 or 2.6, or
82
features deprecated in Python 3.0. (You can check v3 compatibility using
83
the ``-3`` option of Python2.6.)
81
support Python 2.7 and 3.0. Avoid using language features added in 2.5,
82
2.6 or 2.7, or features deprecated in Python 3.0. (You can check v3
83
compatibility using the ``-3`` option of Python2.6.)
87
* Don't use the ``with`` statement.
89
* Don't ``from . import``.
91
* Don't use ``try/except/finally``, which is not supported in Python2.4,
92
use separate nested ``try/except`` and ``try/finally`` blocks.
87
* Don't use the ``with`` statement.
89
* Don't ``from . import``.
91
* Don't use ``try/except/finally``, which is not supported in Python2.4,
92
use separate nested ``try/except`` and ``try/finally`` blocks.
95
95
hasattr and getattr
101
101
if getattr(thing, 'name', None) is None
107
``**kwargs`` in the prototype of a function should be used sparingly.
108
It can be good on higher-order functions that decorate other functions,
109
such as ``addCleanup`` or ``assertRaises``, or on functions that take only
110
(or almost only) kwargs, where any kwargs can be passed.
112
Otherwise, be careful: if the parameters to a function are a bit complex
113
and might vary over time (e.g. the ``commit`` API) then we prefer to pass an
114
object rather than a bag of positional and/or keyword args. If you have
115
an arbitrary set of keys and values that are different with each use (e.g.
116
string interpolation inputs) then again that should not be mixed in with
117
the regular positional/keyword args, it seems like a different category of
121
Imitating standard objects
122
==========================
124
Don't provide methods that imitate built-in classes (eg ``__in__``,
125
``__call__``, ``__int__``, ``__getitem__``) unless the class you're
126
implementing really does act like the builtin class, in semantics and
129
For example, old code lets you say ``file_id in inv`` but we no longer
130
consider this good style. Instead, say more explicitly
131
``inv.has_id(file_id)``.
133
``__repr__``, ``__cmp__``, ``__str__`` are usually fine.
152
184
later time, or possibly never at all. Therefore we have restrictions on
153
185
what can be done inside them.
155
0. If you think you need to use a ``__del__`` method ask another
156
developer for alternatives. If you do need to use one, explain
159
1. Never rely on a ``__del__`` method running. If there is code that
160
must run, do it from a ``finally`` block instead.
162
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
165
3. In some places we raise a warning from the destructor if the object
166
has not been cleaned up or closed. This is considered OK: the warning
167
may not catch every case but it's still useful sometimes.
187
0. If you think you need to use a ``__del__`` method ask another
188
developer for alternatives. If you do need to use one, explain
191
1. Never rely on a ``__del__`` method running. If there is code that
192
must run, do it from a ``finally`` block instead.
194
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
197
3. In some places we raise a warning from the destructor if the object
198
has not been cleaned up or closed. This is considered OK: the warning
199
may not catch every case but it's still useful sometimes.
173
Often when something has failed later code, including cleanups invoked
174
from ``finally`` blocks, will fail too. These secondary failures are
175
generally uninteresting compared to the original exception. So use the
176
``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
177
are typically called in ``finally`` blocks, such as ``unlock`` methods.
178
For example, ``@only_raises(LockNotHeld, LockBroken)``. All errors that
179
are unlikely to be a knock-on failure from a previous failure should be
205
Often when something has failed later code will fail too, including
206
cleanups invoked from ``finally`` blocks. These secondary failures are
207
generally uninteresting compared to the original exception. ``bzrlib``
208
has some facilities you can use to mitigate this.
210
* In ``Command`` subclasses, prefer the ``add_cleanup`` method to using
211
``try``/``finally`` blocks. E.g. to acquire a lock and ensure it will
212
always be released when the command is done::
214
self.add_cleanup(branch.lock_read().unlock)
216
This also avoids heavily indented code. It also makes it easier to notice
217
mismatched lock/unlock pairs (and other kinds of resource
218
acquire/release) because there isn't a large block of code separating
221
* Use the ``only_raises`` decorator (from ``bzrlib.decorators``) when
222
defining methods that are typically called in ``finally`` blocks, such
223
as ``unlock`` methods. For example, ``@only_raises(LockNotHeld,
224
LockBroken)``. All errors that are unlikely to be a knock-on failure
225
from a previous failure should be allowed.
227
* Consider using the ``OperationWithCleanups`` helper from
228
``bzrlib.cleanup`` anywhere else you have a ``finally`` block that
336
387
``Exception`` (which excludes system errors in Python2.5 and later) would
390
The ``__str__`` method on exceptions should be small and have no side
391
effects, following the rules given for `Object string representations`_.
392
In particular it should not do any network IO, or complicated
393
introspection of other objects. All the state needed to present the
394
exception to the user should be gathered before the error is raised.
395
In other words, exceptions should basically be value objects.
357
* It makes the behaviour vary depending on whether bzr is run with -O
358
or not, therefore giving a chance for bugs that occur in one case or
359
the other, several of which have already occurred: assertions with
360
side effects, code which can't continue unless the assertion passes,
361
cases where we should give the user a proper message rather than an
363
* It's not that much shorter than an explicit if/raise.
364
* It tends to lead to fuzzy thinking about whether the check is
365
actually needed or not, and whether it's an internal error or not
366
* It tends to cause look-before-you-leap patterns.
367
* It's unsafe if the check is needed to protect the integrity of the
369
* It tends to give poor messages since the developer can get by with
370
no explanatory text at all.
371
* We can't rely on people always running with -O in normal use, so we
372
can't use it for tests that are actually expensive.
373
* Expensive checks that help developers are better turned on from the
374
test suite or a -D flag.
375
* If used instead of ``self.assert*()`` in tests it makes them falsely pass with -O.
415
* It makes the behaviour vary depending on whether bzr is run with -O
416
or not, therefore giving a chance for bugs that occur in one case or
417
the other, several of which have already occurred: assertions with
418
side effects, code which can't continue unless the assertion passes,
419
cases where we should give the user a proper message rather than an
421
* It's not that much shorter than an explicit if/raise.
422
* It tends to lead to fuzzy thinking about whether the check is
423
actually needed or not, and whether it's an internal error or not
424
* It tends to cause look-before-you-leap patterns.
425
* It's unsafe if the check is needed to protect the integrity of the
427
* It tends to give poor messages since the developer can get by with
428
no explanatory text at all.
429
* We can't rely on people always running with -O in normal use, so we
430
can't use it for tests that are actually expensive.
431
* Expensive checks that help developers are better turned on from the
432
test suite or a -D flag.
433
* If used instead of ``self.assert*()`` in tests it makes them falsely
489
Bazaar is a GNU project and uses standard GNU terminology, especially:
491
* Use the word "Linux" to refer to the Linux kernel, not as a synechoche
492
for the entire operating system. (See `bug 528253
493
<https://bugs.launchpad.net/bzr/+bug/528253>`_).
495
* Don't say "open source" when you mean "free software".
501
If you need to import a module (or attribute of a module) named in a
504
* If importing a module, not an attribute, and the module is a top-level
505
module (i.e. has no dots in the name), then it's ok to use the builtin
506
``__import__``, e.g. ``__import__(module_name)``.
507
* In all other cases, prefer ``bzrlib.pyutils.get_named_object`` to the
508
built-in ``__import__``. ``__import__`` has some subtleties and
509
unintuitive behaviours that make it hard to use correctly.
427
512
vim: ft=rst tw=74 ai
added
removed
doc/developers/code-style.txt
