974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
1 |
============================
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
2 |
Guidelines for modifying bzr |
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
3 |
============================
|
4 |
||
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
5 |
.. contents:: |
6 |
||
7 |
(The current version of this document is available in the file ``HACKING`` |
|
8 |
in the source tree, or at http://bazaar-ng.org/hacking.html) |
|
9 |
||
10 |
Overall
|
|
11 |
=======
|
|
12 |
||
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
13 |
* New functionality should have test cases. Preferably write the |
14 |
test before writing the code. |
|
15 |
||
16 |
In general, you can test at either the command-line level or the |
|
17 |
internal API level. Choose whichever is appropriate: if adding a |
|
18 |
new command, or a new command option, then call through run_bzr(). |
|
1412
by Robert Collins
update HACKING |
19 |
It is not necessary to do both. Tests that test the command line level |
20 |
are appropriate for checking the UI behaves well - bug fixes and |
|
21 |
core improvements should be tested closer to the code that is doing the |
|
22 |
work. Command line level tests should be placed in 'blackbox.py'. |
|
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
23 |
|
24 |
* Before fixing a bug, write a test case so that it does not regress. |
|
25 |
||
26 |
* Exceptions should be defined inside bzrlib.errors, so that we can |
|
27 |
see the whole tree at a glance. |
|
28 |
||
29 |
* Imports should be done at the top-level of the file, unless there is |
|
30 |
a strong reason to have them lazily loaded when a particular |
|
31 |
function runs. Import statements have a cost, so try to make sure |
|
32 |
they don't run inside hot functions. |
|
33 |
||
34 |
* Module names should always be given fully-qualified,
|
|
35 |
i.e. ``bzrlib.hashcache`` not just ``hashcache``.
|
|
36 |
||
1476
by Robert Collins
Merge now has a retcode of 1 when conflicts occur. (Robert Collins) |
37 |
* Commands should return Non-Zero when they encounter circumstances that
|
38 |
the user should really pay attention to - which includes trivial shell
|
|
39 |
pipelines.
|
|
40 |
||
41 |
Recommanded values are
|
|
42 |
0- OK,
|
|
43 |
1- Conflicts in merge-like operations, or changes are present in
|
|
44 |
diff-like operations.
|
|
45 |
2- Unrepresentable diff changes (i.e. binary files that we cannot show
|
|
46 |
a diff of).
|
|
47 |
3- An error or exception has occured.
|
|
48 |
||
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
49 |
Evolving interfaces
|
50 |
-------------------
|
|
51 |
||
52 |
If you change the behaviour of an API in an incompatible way, please
|
|
53 |
be sure to change its name as well. For instance, if I add a keyword
|
|
54 |
parameter to branch.commit - that's fine. On the other hand, if I add |
|
55 |
a keyword parameter to branch.commit which is a *required* transaction |
|
56 |
object, I should rename the api - i.e. to 'branch.commit_transaction'. |
|
57 |
||
58 |
This will prevent users of the old api getting surprising results. |
|
59 |
Instead, they will get an Attribute error as the api is missing, and |
|
60 |
will know to update their code. If in doubt, just ask on #bzr. |
|
61 |
||
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
62 |
Documentation
|
63 |
=============
|
|
64 |
||
65 |
If you change the behaviour of a command, please update its docstring |
|
66 |
in bzrlib/commands.py. This is displayed by the 'bzr help' command. |
|
67 |
||
68 |
If you make a user-visible change, please add a note to the NEWS file. |
|
69 |
The description should be written to make sense to someone who's just |
|
70 |
a user of bzr, not a developer: new functions or classes shouldn't be |
|
71 |
mentioned, but new commands, changes in behaviour or fixed nontrivial |
|
72 |
bugs should be listed. See the existing entries for an idea of what |
|
73 |
should be done. |
|
1098
by Martin Pool
- notes on how output is written |
74 |
|
75 |
||
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
76 |
API documentation |
77 |
-----------------
|
|
78 |
||
79 |
Functions, methods, classes and modules should have docstrings |
|
80 |
describing how they are used. |
|
81 |
||
82 |
The first line of the docstring should be a self-contained sentence. |
|
83 |
||
84 |
For the special case of Command classes, this acts as the user-visible |
|
85 |
documentation shown by the help command. |
|
86 |
||
87 |
The docstrings should be formatted as reStructuredText_ (like this |
|
88 |
document), suitable for processing using the epydoc_ tool into HTML |
|
89 |
documentation. |
|
90 |
||
91 |
.. _reStructuredText: http://docutils.sourceforge.net/rst.html |
|
92 |
.. _epydoc: http://epydoc.sourceforge.net/ |
|
93 |
||
94 |
||
95 |
||
96 |
Coding style |
|
97 |
============
|
|
98 |
||
99 |
Please write PEP-8__ compliant code. |
|
100 |
||
101 |
One often-missed requirement is that the first line of docstrings |
|
102 |
should be a self-contained one-sentence summary. |
|
103 |
||
104 |
__ http://www.python.org/peps/pep-0008.html |
|
105 |
||
106 |
||
107 |
||
108 |
Naming
|
|
109 |
------
|
|
110 |
||
111 |
Functions, methods or members that are in some sense "private" are given |
|
112 |
a leading underscore prefix. This is just a hint that code outside the |
|
113 |
implementation should probably not use that interface. |
|
114 |
||
115 |
We prefer class names to be concatenated capital words (``TestCase``) |
|
116 |
and variables, methods and functions to be lowercase words joined by |
|
117 |
underscores (``revision_id``, ``get_revision``). |
|
118 |
||
119 |
For the purposes of naming some names are treated as single compound |
|
120 |
words: "filename", "revno". |
|
121 |
||
122 |
Consider naming classes as nouns and functions/methods as verbs. |
|
123 |
||
124 |
||
125 |
Standard names |
|
126 |
--------------
|
|
127 |
||
128 |
``revision_id`` not ``rev_id`` or ``revid`` |
|
129 |
||
130 |
Functions that transform one thing to another should be named ``x_to_y`` |
|
131 |
(not ``x2y`` as occurs in some old code.) |
|
132 |
||
1098
by Martin Pool
- notes on how output is written |
133 |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
134 |
Destructors
|
135 |
-----------
|
|
136 |
||
1185.16.150
by Martin Pool
Improved description of python exception policies |
137 |
Python destructors (``__del__``) work differently to those of other |
138 |
languages. In particular, bear in mind that destructors may be called |
|
139 |
immediately when the object apparently becomes unreferenced, or at some |
|
140 |
later time, or possibly never at all. Therefore we have restrictions on |
|
141 |
what can be done inside them. |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
142 |
|
143 |
0. Never use a __del__ method without asking Martin/Robert first. |
|
144 |
||
145 |
1. Never rely on a ``__del__`` method running. If there is code that |
|
146 |
must run, do it from a ``finally`` block instead. |
|
147 |
||
148 |
2. Never ``import`` from inside a ``__del__`` method, or you may crash the |
|
149 |
interpreter!! |
|
150 |
||
151 |
3. In some places we raise a warning from the destructor if the object |
|
152 |
has not been cleaned up or closed. This is considered OK: the warning |
|
153 |
may not catch every case but it's still useful sometimes. |
|
154 |
||
155 |
||
1098
by Martin Pool
- notes on how output is written |
156 |
Writing output
|
157 |
==============
|
|
158 |
||
159 |
(The strategy described here is what we want to get to, but it's not |
|
160 |
consistently followed in the code at the moment.) |
|
161 |
||
162 |
bzrlib is intended to be a generically reusable library. It shouldn't |
|
163 |
write messages to stdout or stderr, because some programs that use it
|
|
164 |
might want to display that information through a GUI or some other
|
|
165 |
mechanism.
|
|
166 |
||
167 |
We can distinguish two types of output from the library:
|
|
168 |
||
169 |
1. Structured data representing the progress or result of an
|
|
170 |
operation. For example, for a commit command this will be a list
|
|
171 |
of the modified files and the finally committed revision number
|
|
172 |
and id.
|
|
173 |
||
174 |
These should be exposed either through the return code or by calls
|
|
175 |
to a callback parameter.
|
|
176 |
||
177 |
A special case of this is progress indicators for long-lived
|
|
178 |
operations, where the caller should pass a ProgressBar object.
|
|
179 |
||
180 |
2. Unstructured log/debug messages, mostly for the benefit of the
|
|
181 |
developers or users trying to debug problems. This should always
|
|
182 |
be sent through ``bzrlib.trace`` and Python ``logging``, so that
|
|
183 |
it can be redirected by the client.
|
|
184 |
||
185 |
The distinction between the two is a bit subjective, but in general if
|
|
186 |
there is any chance that a library would want to see something as
|
|
187 |
structured data, we should make it so.
|
|
188 |
||
189 |
The policy about how output is presented in the text-mode client
|
|
190 |
should be only in the command-line tool.
|
|
1092.1.22
by Robert Collins
update hacking with some test foo |
191 |
|
1418
by Robert Collins
merge martins latest |
192 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
193 |
Writing tests
|
194 |
=============
|
|
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
195 |
In general tests should be placed in a file named testFOO.py where
|
1092.1.22
by Robert Collins
update hacking with some test foo |
196 |
FOO is the logical thing under test. That file should be placed in the
|
197 |
tests subdirectory under the package being tested.
|
|
198 |
||
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
199 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py.
|
1417.1.2
by Robert Collins
add sample test |
200 |
See bzrlib/selftest/testsampler.py for a template test script.
|
1092.1.22
by Robert Collins
update hacking with some test foo |
201 |
|
1393.1.61
by Martin Pool
doc |
202 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
203 |
Running tests
|
204 |
=============
|
|
205 |
Currently, bzr selftest is used to invoke tests.
|
|
206 |
You can provide a pattern argument to run a subset. For example,
|
|
1393.1.61
by Martin Pool
doc |
207 |
to run just the whitebox tests, run::
|
208 |
||
209 |
bzr selftest -v whitebox
|
|
210 |
||
211 |
||
212 |
Errors and exceptions
|
|
213 |
=====================
|
|
214 |
||
1185.16.61
by mbp at sourcefrog
- start introducing hct error classes |
215 |
Errors are handled through Python exceptions. They can represent user
|
216 |
errors, environmental errors or program bugs. Sometimes we can't be sure |
|
217 |
at the time it's raised which case applies. See bzrlib/errors.py for |
|
218 |
details on the error-handling practices.
|
|
1092.1.22
by Robert Collins
update hacking with some test foo |
219 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
220 |
|
221 |
Jargon
|
|
222 |
======
|
|
223 |
||
224 |
revno
|
|
225 |
Integer identifier for a revision on the main line of a branch.
|
|
226 |
Revision 0 is always the null revision; others are 1-based
|
|
227 |
indexes into the branch's revision history. |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
228 |
|
229 |
:: vim: tw=74 ai |