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
152
152
later time, or possibly never at all. Therefore we have restrictions on
153
153
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.
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.
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
173
Often when something has failed later code will fail too, including
174
cleanups invoked from ``finally`` blocks. These secondary failures are
175
generally uninteresting compared to the original exception. ``bzrlib``
176
has some facilities you can use to mitigate this.
178
* In ``Command`` subclasses, prefer the ``add_cleanup`` method to using
179
``try``/``finally`` blocks. E.g. to acquire a lock and ensure it will
180
always be released when the command is done::
182
self.add_cleanup(branch.lock_read().unlock)
184
This also avoids heavily indented code. It also makes it easier to notice
185
mismatched lock/unlock pairs (and other kinds of resource
186
acquire/release) because there isn't a large block of code separating
189
* Use the ``only_raises`` decorator (from ``bzrlib.decorators``) when
190
defining methods that are typically called in ``finally`` blocks, such
191
as ``unlock`` methods. For example, ``@only_raises(LockNotHeld,
192
LockBroken)``. All errors that are unlikely to be a knock-on failure
193
from a previous failure should be allowed.
195
* Consider using the ``OperationWithCleanups`` helper from
196
``bzrlib.cleanup`` anywhere else you have a ``finally`` block that
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.
374
* It makes the behaviour vary depending on whether bzr is run with -O
375
or not, therefore giving a chance for bugs that occur in one case or
376
the other, several of which have already occurred: assertions with
377
side effects, code which can't continue unless the assertion passes,
378
cases where we should give the user a proper message rather than an
380
* It's not that much shorter than an explicit if/raise.
381
* It tends to lead to fuzzy thinking about whether the check is
382
actually needed or not, and whether it's an internal error or not
383
* It tends to cause look-before-you-leap patterns.
384
* It's unsafe if the check is needed to protect the integrity of the
386
* It tends to give poor messages since the developer can get by with
387
no explanatory text at all.
388
* We can't rely on people always running with -O in normal use, so we
389
can't use it for tests that are actually expensive.
390
* Expensive checks that help developers are better turned on from the
391
test suite or a -D flag.
392
* If used instead of ``self.assert*()`` in tests it makes them falsely
added
removed
doc/developers/code-style.txt
