1
***********************
2
Breezy Code Style Guide
3
***********************
8
Please write PEP-8__ compliant code.
10
__ http://www.python.org/peps/pep-0008.html
12
One often-missed requirement is that the first line of docstrings
13
should be a self-contained one-sentence summary.
15
We use 4 space indents for blocks, and never use tab characters. (In vim,
18
Trailing white space should be avoided, but is allowed.
19
You should however not make lots of unrelated white space changes.
21
Unix style newlines (LF) are used.
23
Each file must have a newline at the end of it.
25
Lines should be no more than 79 characters if at all possible.
26
Lines that continue a long statement may be indented in either of
29
within the parenthesis or other character that opens the block, e.g.::
35
or indented by four spaces::
41
The first is considered clearer by some people; however it can be a bit
42
harder to maintain (e.g. when the method name changes), and it does not
43
work well if the relevant parenthesis is already far to the right. Avoid
46
self.legbone.kneebone.shinbone.toebone.shake_it(one,
52
self.legbone.kneebone.shinbone.toebone.shake_it(one,
58
self.legbone.kneebone.shinbone.toebone.shake_it(
61
For long lists, we like to add a trailing comma and put the closing
62
character on the following line. This makes it easier to add new items in
65
from breezy.goo import (
71
There should be spaces between function parameters, but not between the
72
keyword name and the value::
74
call(1, 3, cheese=quark)
80
Breezy supports Python 2.7 and Python 3.5 or later.
85
``hasattr`` should not be used because it swallows exceptions including
86
``KeyboardInterrupt``. Instead, say something like ::
88
if getattr(thing, 'name', None) is None
94
``**kwargs`` in the prototype of a function should be used sparingly.
95
It can be good on higher-order functions that decorate other functions,
96
such as ``addCleanup`` or ``assertRaises``, or on functions that take only
97
(or almost only) kwargs, where any kwargs can be passed.
99
Otherwise, be careful: if the parameters to a function are a bit complex
100
and might vary over time (e.g. the ``commit`` API) then we prefer to pass an
101
object rather than a bag of positional and/or keyword args. If you have
102
an arbitrary set of keys and values that are different with each use (e.g.
103
string interpolation inputs) then again that should not be mixed in with
104
the regular positional/keyword args, it seems like a different category of
108
Imitating standard objects
109
==========================
111
Don't provide methods that imitate built-in classes (eg ``__in__``,
112
``__call__``, ``__int__``, ``__getitem__``) unless the class you're
113
implementing really does act like the builtin class, in semantics and
116
For example, old code lets you say ``file_id in inv`` but we no longer
117
consider this good style. Instead, say more explicitly
118
``inv.has_id(file_id)``.
120
``__repr__``, ``__cmp__``, ``__str__`` are usually fine.
126
* Imports should be done at the top-level of the file, unless there is
127
a strong reason to have them lazily loaded when a particular
128
function runs. Import statements have a cost, so try to make sure
129
they don't run inside hot functions.
131
* Module names should always be given fully-qualified,
132
i.e. ``breezy.hashcache`` not just ``hashcache``.
138
Functions, methods or members that are relatively private are given
139
a leading underscore prefix. Names without a leading underscore are
140
public not just across modules but to programmers using breezy as an
143
We prefer class names to be concatenated capital words (``TestCase``)
144
and variables, methods and functions to be lowercase words joined by
145
underscores (``revision_id``, ``get_revision``).
147
For the purposes of naming some names are treated as single compound
148
words: "filename", "revno".
150
Consider naming classes as nouns and functions/methods as verbs.
152
Try to avoid using abbreviations in names, because there can be
153
inconsistency if other people use the full name.
159
``revision_id`` not ``rev_id`` or ``revid``
161
Functions that transform one thing to another should be named ``x_to_y``
162
(not ``x2y`` as occurs in some old code.)
168
Python destructors (``__del__``) work differently to those of other
169
languages. In particular, bear in mind that destructors may be called
170
immediately when the object apparently becomes unreferenced, or at some
171
later time, or possibly never at all. Therefore we have restrictions on
172
what can be done inside them.
174
0. If you think you need to use a ``__del__`` method ask another
175
developer for alternatives. If you do need to use one, explain
178
1. Never rely on a ``__del__`` method running. If there is code that
179
must run, instead have a ``finally`` block or an ``addCleanup`` call an
180
explicit ``close`` method.
182
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
185
3. Prior to bzr 2.4, we sometimes used to raise warnings from del methods
186
that the object was not cleaned up or closed. We no longer do this:
187
failure to close the object doesn't cause a test failure; the warning
188
appears an arbitrary long time after the problem occurred (the object
189
being leaked); merely having a del method inhibits Python gc; the
190
warnings appear to users and upset them; they can also break tests that
191
are checking what appears on stderr.
193
In short, just don't use ``__del__``.
198
Often when something has failed later code will fail too, including
199
cleanups invoked from ``finally`` blocks. These secondary failures are
200
generally uninteresting compared to the original exception. ``breezy``
201
has some facilities you can use to mitigate this.
203
* In ``Command`` subclasses, prefer the ``add_cleanup`` method to using
204
``try``/``finally`` blocks. E.g. to acquire a lock and ensure it will
205
always be released when the command is done::
207
self.add_cleanup(branch.lock_read().unlock)
209
This also avoids heavily indented code. It also makes it easier to notice
210
mismatched lock/unlock pairs (and other kinds of resource
211
acquire/release) because there isn't a large block of code separating
214
* Use the ``only_raises`` decorator (from ``breezy.decorators``) when
215
defining methods that are typically called in ``finally`` blocks, such
216
as ``unlock`` methods. For example, ``@only_raises(LockNotHeld,
217
LockBroken)``. All errors that are unlikely to be a knock-on failure
218
from a previous failure should be allowed.
220
* Consider using the ``OperationWithCleanups`` helper from
221
``breezy.cleanup`` anywhere else you have a ``finally`` block that
228
In some places we have variables which point to callables that construct
229
new instances. That is to say, they can be used a lot like class objects,
230
but they shouldn't be *named* like classes. Things called ``FooBar`` should
231
create an instance of ``FooBar``. A factory method that might create a
232
``FooBar`` or might make something else should be called ``foo_factory``.
238
Several places in Breezy use (or will use) a registry, which is a
239
mapping from names to objects or classes. The registry allows for
240
loading in registered code only when it's needed, and keeping
241
associated information such as a help string or description.
244
InterObject and multiple dispatch
245
=================================
247
The ``InterObject`` provides for two-way `multiple dispatch`__: matching
248
up for example a source and destination repository to find the right way
249
to transfer data between them.
251
.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
253
There is a subclass ``InterObject`` classes for each type of object that is
254
dispatched this way, e.g. ``InterRepository``. Calling ``.get()`` on this
255
class will return an ``InterObject`` instance providing the best match for
256
those parameters, and this instance then has methods for operations
261
inter = InterRepository.get(source_repo, target_repo)
262
inter.fetch(revision_id)
264
``InterRepository`` also acts as a registry-like object for its
265
subclasses, and they can be added through ``.register_optimizer``. The
266
right one to run is selected by asking each class, in reverse order of
267
registration, whether it ``.is_compatible`` with the relevant objects.
272
To make startup time faster, we use the ``breezy.lazy_import`` module to
273
delay importing modules until they are actually used. ``lazy_import`` uses
274
the same syntax as regular python imports. So to import a few modules in a
277
from breezy.lazy_import import lazy_import
278
lazy_import(globals(), """
287
revision as _mod_revision,
289
import breezy.transport
293
At this point, all of these exist as a ``ImportReplacer`` object, ready to
294
be imported once a member is accessed. Also, when importing a module into
295
the local namespace, which is likely to clash with variable names, it is
296
recommended to prefix it as ``_mod_<module>``. This makes it clearer that
297
the variable is a module, and these object should be hidden anyway, since
298
they shouldn't be imported into other namespaces.
300
While it is possible for ``lazy_import()`` to import members of a module
301
when using the ``from module import member`` syntax, it is recommended to
302
only use that syntax to load sub modules ``from module import submodule``.
303
This is because variables and classes can frequently be used without
304
needing a sub-member for example::
306
lazy_import(globals(), """
307
from module import MyClass
311
return isinstance(x, MyClass)
313
This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
314
object, rather than the real class.
316
It also is incorrect to assign ``ImportReplacer`` objects to other variables.
317
Because the replacer only knows about the original name, it is unable to
318
replace other variables. The ``ImportReplacer`` class will raise an
319
``IllegalUseOfScopeReplacer`` exception if it can figure out that this
320
happened. But it requires accessing a member more than once from the new
321
variable, so some bugs are not detected right away.
327
The null revision is the ancestor of all revisions. Its revno is 0, its
328
revision-id is ``null:``, and its tree is the empty tree. When referring
329
to the null revision, please use ``breezy.revision.NULL_REVISION``. Old
330
code sometimes uses ``None`` for the null revision, but this practice is
334
Object string representations
335
=============================
337
Python prints objects using their ``__repr__`` method when they are
338
written to logs, exception tracebacks, or the debugger. We want
339
objects to have useful representations to help in determining what went
342
If you add a new class you should generally add a ``__repr__`` method
343
unless there is an adequate method in a parent class. There should be a
346
Representations should typically look like Python constructor syntax, but
347
they don't need to include every value in the object and they don't need
348
to be able to actually execute. They're to be read by humans, not
349
machines. Don't hardcode the classname in the format, so that we get the
350
correct value if the method is inherited by a subclass. If you're
351
printing attributes of the object, including strings, you should normally
352
use ``%r`` syntax (to call their repr in turn).
354
Try to avoid the representation becoming more than one or two lines long.
355
(But balance this against including useful information, and simplicity of
358
Because repr methods are often called when something has already gone
359
wrong, they should be written somewhat more defensively than most code.
360
They shouldn't have side effects like doing network or disk
362
The object may be half-initialized or in some other way in an illegal
363
state. The repr method shouldn't raise an exception, or it may hide the
364
(probably more useful) underlying exception.
369
return '%s(%r)' % (self.__class__.__name__,
376
A bare ``except`` statement will catch all exceptions, including ones that
377
really should terminate the program such as ``MemoryError`` and
378
``KeyboardInterrupt``. They should rarely be used unless the exception is
379
later re-raised. Even then, think about whether catching just
380
``Exception`` (which excludes system errors in Python2.5 and later) would
383
The ``__str__`` method on exceptions should be small and have no side
384
effects, following the rules given for `Object string representations`_.
385
In particular it should not do any network IO, or complicated
386
introspection of other objects. All the state needed to present the
387
exception to the user should be gathered before the error is raised.
388
In other words, exceptions should basically be value objects.
394
All code should be exercised by the test suite. See the `Breezy Testing
395
Guide <http://www.breezy-vcs.org/developers/testing.html>`_ for detailed
396
information about writing tests.
402
Do not use the Python ``assert`` statement, either in tests or elsewhere.
403
A source test checks that it is not used. It is ok to explicitly raise
408
* It makes the behaviour vary depending on whether brz is run with -O
409
or not, therefore giving a chance for bugs that occur in one case or
410
the other, several of which have already occurred: assertions with
411
side effects, code which can't continue unless the assertion passes,
412
cases where we should give the user a proper message rather than an
414
* It's not that much shorter than an explicit if/raise.
415
* It tends to lead to fuzzy thinking about whether the check is
416
actually needed or not, and whether it's an internal error or not
417
* It tends to cause look-before-you-leap patterns.
418
* It's unsafe if the check is needed to protect the integrity of the
420
* It tends to give poor messages since the developer can get by with
421
no explanatory text at all.
422
* We can't rely on people always running with -O in normal use, so we
423
can't use it for tests that are actually expensive.
424
* Expensive checks that help developers are better turned on from the
425
test suite or a -D flag.
426
* If used instead of ``self.assert*()`` in tests it makes them falsely
434
;(defface my-invalid-face
435
; '((t (:background "Red" :underline t)))
436
; "Face used to highlight invalid constructs or other uglyties"
439
(defun my-python-mode-hook ()
440
;; setup preferred indentation style.
441
(setq fill-column 79)
442
(setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
443
; (font-lock-add-keywords 'python-mode
444
; '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
445
; ("[ \t]+$" . 'my-invalid-face) ; Trailing spaces
446
; ("^[ \t]+$" . 'my-invalid-face)); Spaces only
450
(add-hook 'python-mode-hook 'my-python-mode-hook)
452
The lines beginning with ';' are comments. They can be activated
453
if one want to have a strong notice of some tab/space usage
459
The ``breezy.osutils`` module has many useful helper functions, including
460
some more portable variants of functions in the standard library.
462
In particular, don't use ``shutil.rmtree`` unless it's acceptable for it
463
to fail on Windows if some files are readonly or still open elsewhere.
464
Use ``breezy.osutils.rmtree`` instead.
466
Using the ``open(..).read(..)`` or ``open(..).write(..)`` style chaining
467
of methods for reading or writing file content relies on garbage collection
468
to close the file which may keep the file open for an undefined period of
469
time. This may break some follow up operations like rename on Windows.
470
Use ``try/finally`` to explictly close the file. E.g.::
472
f = open('foo.txt', 'w')
482
If you need to import a module (or attribute of a module) named in a
485
* If importing a module, not an attribute, and the module is a top-level
486
module (i.e. has no dots in the name), then it's ok to use the builtin
487
``__import__``, e.g. ``__import__(module_name)``.
488
* In all other cases, prefer ``breezy.pyutils.get_named_object`` to the
489
built-in ``__import__``. ``__import__`` has some subtleties and
490
unintuitive behaviours that make it hard to use correctly.
added
removed
doc/developers/code-style.txt
