« back to all changes in this revision
Viewing changes to doc/developers/code-style.txt
- browse files at revision 5225.2.9
- compare with another revision
- compare with revision 1012
- download diff
- download tarball
- view history from revision 5225.2.9
- Committer: Martin Pool
- Date: 2010-05-15 10:57:30 UTC
- mto: This revision was merged to the branch mainline in revision 5252.
- Revision ID: mbp@canonical.com-20100515105730-5iwm6llngk954qi8
Split out code style guide from HACKING
- files added:
- doc/developers/code-style.txt
- files modified:
- doc/developers/HACKING.txt
added
removed
doc/developers/code-style.txt
1
***********************
2
Bazaar Code Style Guide
3
***********************
4
5
hasattr and getattr
6
===================
7
8
``hasattr`` should not be used because it swallows exceptions including
9
``KeyboardInterrupt``. Instead, say something like ::
10
11
if getattr(thing, 'name', None) is None
12
13
14
Code layout
15
===========
16
17
Please write PEP-8__ compliant code.
18
19
__ http://www.python.org/peps/pep-0008.html
20
21
One often-missed requirement is that the first line of docstrings
22
should be a self-contained one-sentence summary.
23
24
We use 4 space indents for blocks, and never use tab characters. (In vim,
25
``set expandtab``.)
26
27
Trailing white space should be avoided, but is allowed.
28
You should however not make lots of unrelated white space changes.
29
30
Unix style newlines (LF) are used.
31
32
Each file must have a newline at the end of it.
33
34
Lines should be no more than 79 characters if at all possible.
35
Lines that continue a long statement may be indented in either of
36
two ways:
37
38
within the parenthesis or other character that opens the block, e.g.::
39
40
my_long_method(arg1,
41
arg2,
42
arg3)
43
44
or indented by four spaces::
45
46
my_long_method(arg1,
47
arg2,
48
arg3)
49
50
The first is considered clearer by some people; however it can be a bit
51
harder to maintain (e.g. when the method name changes), and it does not
52
work well if the relevant parenthesis is already far to the right. Avoid
53
this::
54
55
self.legbone.kneebone.shinbone.toebone.shake_it(one,
56
two,
57
three)
58
59
but rather ::
60
61
self.legbone.kneebone.shinbone.toebone.shake_it(one,
62
two,
63
three)
64
65
or ::
66
67
self.legbone.kneebone.shinbone.toebone.shake_it(
68
one, two, three)
69
70
For long lists, we like to add a trailing comma and put the closing
71
character on the following line. This makes it easier to add new items in
72
future::
73
74
from bzrlib.goo import (
75
jam,
76
jelly,
77
marmalade,
78
)
79
80
There should be spaces between function parameters, but not between the
81
keyword name and the value::
82
83
call(1, 3, cheese=quark)
84
85
In emacs::
86
87
;(defface my-invalid-face
88
; '((t (:background "Red" :underline t)))
89
; "Face used to highlight invalid constructs or other uglyties"
90
; )
91
92
(defun my-python-mode-hook ()
93
;; setup preferred indentation style.
94
(setq fill-column 79)
95
(setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
96
; (font-lock-add-keywords 'python-mode
97
; '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
98
; ("[ \t]+$" . 'my-invalid-face) ; Trailing spaces
99
; ("^[ \t]+$" . 'my-invalid-face)); Spaces only
100
; )
101
)
102
103
(add-hook 'python-mode-hook 'my-python-mode-hook)
104
105
The lines beginning with ';' are comments. They can be activated
106
if one want to have a strong notice of some tab/space usage
107
violations.
108
109
110
Module Imports
111
==============
112
113
* Imports should be done at the top-level of the file, unless there is
114
a strong reason to have them lazily loaded when a particular
115
function runs. Import statements have a cost, so try to make sure
116
they don't run inside hot functions.
117
118
* Module names should always be given fully-qualified,
119
i.e. ``bzrlib.hashcache`` not just ``hashcache``.
120
121
122
Naming
123
======
124
125
Functions, methods or members that are relatively private are given
126
a leading underscore prefix. Names without a leading underscore are
127
public not just across modules but to programmers using bzrlib as an
128
API.
129
130
We prefer class names to be concatenated capital words (``TestCase``)
131
and variables, methods and functions to be lowercase words joined by
132
underscores (``revision_id``, ``get_revision``).
133
134
For the purposes of naming some names are treated as single compound
135
words: "filename", "revno".
136
137
Consider naming classes as nouns and functions/methods as verbs.
138
139
Try to avoid using abbreviations in names, because there can be
140
inconsistency if other people use the full name.
141
142
143
Standard Names
144
==============
145
146
``revision_id`` not ``rev_id`` or ``revid``
147
148
Functions that transform one thing to another should be named ``x_to_y``
149
(not ``x2y`` as occurs in some old code.)
150
151
152
Destructors
153
===========
154
155
Python destructors (``__del__``) work differently to those of other
156
languages. In particular, bear in mind that destructors may be called
157
immediately when the object apparently becomes unreferenced, or at some
158
later time, or possibly never at all. Therefore we have restrictions on
159
what can be done inside them.
160
161
0. If you think you need to use a ``__del__`` method ask another
162
developer for alternatives. If you do need to use one, explain
163
why in a comment.
164
165
1. Never rely on a ``__del__`` method running. If there is code that
166
must run, do it from a ``finally`` block instead.
167
168
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
169
interpreter!!
170
171
3. In some places we raise a warning from the destructor if the object
172
has not been cleaned up or closed. This is considered OK: the warning
173
may not catch every case but it's still useful sometimes.
174
175
176
Cleanup methods
177
===============
178
179
Often when something has failed later code, including cleanups invoked
180
from ``finally`` blocks, will fail too. These secondary failures are
181
generally uninteresting compared to the original exception. So use the
182
``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
183
are typically called in ``finally`` blocks, such as ``unlock`` methods.
184
For example, ``@only_raises(LockNotHeld, LockBroken)``. All errors that
185
are unlikely to be a knock-on failure from a previous failure should be
186
allowed.
187
188
189
Factories
190
=========
191
192
In some places we have variables which point to callables that construct
193
new instances. That is to say, they can be used a lot like class objects,
194
but they shouldn't be *named* like classes::
195
196
> I think that things named FooBar should create instances of FooBar when
197
> called. Its plain confusing for them to do otherwise. When we have
198
> something that is going to be used as a class - that is, checked for via
199
> isinstance or other such idioms, them I would call it foo_class, so that
200
> it is clear that a callable is not sufficient. If it is only used as a
201
> factory, then yes, foo_factory is what I would use.
202
203
204
Registries
205
==========
206
207
Several places in Bazaar use (or will use) a registry, which is a
208
mapping from names to objects or classes. The registry allows for
209
loading in registered code only when it's needed, and keeping
210
associated information such as a help string or description.
211
212
213
InterObject and multiple dispatch
214
=================================
215
216
The ``InterObject`` provides for two-way `multiple dispatch`__: matching
217
up for example a source and destination repository to find the right way
218
to transfer data between them.
219
220
.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
221
222
There is a subclass ``InterObject`` classes for each type of object that is
223
dispatched this way, e.g. ``InterRepository``. Calling ``.get()`` on this
224
class will return an ``InterObject`` instance providing the best match for
225
those parameters, and this instance then has methods for operations
226
between the objects.
227
228
::
229
230
inter = InterRepository.get(source_repo, target_repo)
231
inter.fetch(revision_id)
232
233
``InterRepository`` also acts as a registry-like object for its
234
subclasses, and they can be added through ``.register_optimizer``. The
235
right one to run is selected by asking each class, in reverse order of
236
registration, whether it ``.is_compatible`` with the relevant objects.
237
238
Lazy Imports
239
============
240
241
To make startup time faster, we use the ``bzrlib.lazy_import`` module to
242
delay importing modules until they are actually used. ``lazy_import`` uses
243
the same syntax as regular python imports. So to import a few modules in a
244
lazy fashion do::
245
246
from bzrlib.lazy_import import lazy_import
247
lazy_import(globals(), """
248
import os
249
import subprocess
250
import sys
251
import time
252
253
from bzrlib import (
254
errors,
255
transport,
256
revision as _mod_revision,
257
)
258
import bzrlib.transport
259
import bzrlib.xml5
260
""")
261
262
At this point, all of these exist as a ``ImportReplacer`` object, ready to
263
be imported once a member is accessed. Also, when importing a module into
264
the local namespace, which is likely to clash with variable names, it is
265
recommended to prefix it as ``_mod_<module>``. This makes it clearer that
266
the variable is a module, and these object should be hidden anyway, since
267
they shouldn't be imported into other namespaces.
268
269
While it is possible for ``lazy_import()`` to import members of a module
270
when using the ``from module import member`` syntax, it is recommended to
271
only use that syntax to load sub modules ``from module import submodule``.
272
This is because variables and classes can frequently be used without
273
needing a sub-member for example::
274
275
lazy_import(globals(), """
276
from module import MyClass
277
""")
278
279
def test(x):
280
return isinstance(x, MyClass)
281
282
This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
283
object, rather than the real class.
284
285
It also is incorrect to assign ``ImportReplacer`` objects to other variables.
286
Because the replacer only knows about the original name, it is unable to
287
replace other variables. The ``ImportReplacer`` class will raise an
288
``IllegalUseOfScopeReplacer`` exception if it can figure out that this
289
happened. But it requires accessing a member more than once from the new
290
variable, so some bugs are not detected right away.
291
292
293
The Null revision
294
=================
295
296
The null revision is the ancestor of all revisions. Its revno is 0, its
297
revision-id is ``null:``, and its tree is the empty tree. When referring
298
to the null revision, please use ``bzrlib.revision.NULL_REVISION``. Old
299
code sometimes uses ``None`` for the null revision, but this practice is
300
being phased out.
301
302
303
Object string representations
304
=============================
305
306
Python prints objects using their ``__repr__`` method when they are
307
written to logs, exception tracebacks, or the debugger. We want
308
objects to have useful representations to help in determining what went
309
wrong.
310
311
If you add a new class you should generally add a ``__repr__`` method
312
unless there is an adequate method in a parent class. There should be a
313
test for the repr.
314
315
Representations should typically look like Python constructor syntax, but
316
they don't need to include every value in the object and they don't need
317
to be able to actually execute. They're to be read by humans, not
318
machines. Don't hardcode the classname in the format, so that we get the
319
correct value if the method is inherited by a subclass. If you're
320
printing attributes of the object, including strings, you should normally
321
use ``%r`` syntax (to call their repr in turn).
322
323
Try to avoid the representation becoming more than one or two lines long.
324
(But balance this against including useful information, and simplicity of
325
implementation.)
326
327
Because repr methods are often called when something has already gone
328
wrong, they should be written somewhat more defensively than most code.
329
The object may be half-initialized or in some other way in an illegal
330
state. The repr method shouldn't raise an exception, or it may hide the
331
(probably more useful) underlying exception.
332
333
Example::
334
335
def __repr__(self):
336
return '%s(%r)' % (self.__class__.__name__,
337
self._transport)
338
339
340
Exception handling
341
==================
342
343
A bare ``except`` statement will catch all exceptions, including ones that
344
really should terminate the program such as ``MemoryError`` and
345
``KeyboardInterrupt``. They should rarely be used unless the exception is
346
later re-raised. Even then, think about whether catching just
347
``Exception`` (which excludes system errors in Python2.5 and later) would
348
be better.
349
350
351
Test coverage
352
=============
353
354
All code should be exercised by the test suite. See the `Bazaar Testing
355
Guide <http://doc.bazaar-vcs.org/developers/testing.html>`_ for detailed
356
information about writing tests.
357
Loggerhead is a web-based interface for Breezy
Version: 2.0.1