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 |
||
137 |
Python destructors (``__del__``) work rather differently from in other |
|
138 |
languages. In particular, bear in mind that destructors may not be called |
|
139 |
immediately when the object apparently becomes unreferenced, and that |
|
140 |
there are tight restrictions on what can be done inside them. |
|
141 |
||
142 |
0. Never use a __del__ method without asking Martin/Robert first. |
|
143 |
||
144 |
1. Never rely on a ``__del__`` method running. If there is code that |
|
145 |
must run, do it from a ``finally`` block instead. |
|
146 |
||
147 |
2. Never ``import`` from inside a ``__del__`` method, or you may crash the |
|
148 |
interpreter!! |
|
149 |
||
150 |
3. In some places we raise a warning from the destructor if the object |
|
151 |
has not been cleaned up or closed. This is considered OK: the warning |
|
152 |
may not catch every case but it's still useful sometimes. |
|
153 |
||
154 |
||
1098
by Martin Pool
- notes on how output is written |
155 |
Writing output
|
156 |
==============
|
|
157 |
||
158 |
(The strategy described here is what we want to get to, but it's not |
|
159 |
consistently followed in the code at the moment.) |
|
160 |
||
161 |
bzrlib is intended to be a generically reusable library. It shouldn't |
|
162 |
write messages to stdout or stderr, because some programs that use it
|
|
163 |
might want to display that information through a GUI or some other
|
|
164 |
mechanism.
|
|
165 |
||
166 |
We can distinguish two types of output from the library:
|
|
167 |
||
168 |
1. Structured data representing the progress or result of an
|
|
169 |
operation. For example, for a commit command this will be a list
|
|
170 |
of the modified files and the finally committed revision number
|
|
171 |
and id.
|
|
172 |
||
173 |
These should be exposed either through the return code or by calls
|
|
174 |
to a callback parameter.
|
|
175 |
||
176 |
A special case of this is progress indicators for long-lived
|
|
177 |
operations, where the caller should pass a ProgressBar object.
|
|
178 |
||
179 |
2. Unstructured log/debug messages, mostly for the benefit of the
|
|
180 |
developers or users trying to debug problems. This should always
|
|
181 |
be sent through ``bzrlib.trace`` and Python ``logging``, so that
|
|
182 |
it can be redirected by the client.
|
|
183 |
||
184 |
The distinction between the two is a bit subjective, but in general if
|
|
185 |
there is any chance that a library would want to see something as
|
|
186 |
structured data, we should make it so.
|
|
187 |
||
188 |
The policy about how output is presented in the text-mode client
|
|
189 |
should be only in the command-line tool.
|
|
1092.1.22
by Robert Collins
update hacking with some test foo |
190 |
|
1418
by Robert Collins
merge martins latest |
191 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
192 |
Writing tests
|
193 |
=============
|
|
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
194 |
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 |
195 |
FOO is the logical thing under test. That file should be placed in the
|
196 |
tests subdirectory under the package being tested.
|
|
197 |
||
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
198 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py.
|
1417.1.2
by Robert Collins
add sample test |
199 |
See bzrlib/selftest/testsampler.py for a template test script.
|
1092.1.22
by Robert Collins
update hacking with some test foo |
200 |
|
1393.1.61
by Martin Pool
doc |
201 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
202 |
Running tests
|
203 |
=============
|
|
204 |
Currently, bzr selftest is used to invoke tests.
|
|
205 |
You can provide a pattern argument to run a subset. For example,
|
|
1393.1.61
by Martin Pool
doc |
206 |
to run just the whitebox tests, run::
|
207 |
||
208 |
bzr selftest -v whitebox
|
|
209 |
||
210 |
||
211 |
Errors and exceptions
|
|
212 |
=====================
|
|
213 |
||
1185.16.61
by mbp at sourcefrog
- start introducing hct error classes |
214 |
Errors are handled through Python exceptions. They can represent user
|
215 |
errors, environmental errors or program bugs. Sometimes we can't be sure |
|
216 |
at the time it's raised which case applies. See bzrlib/errors.py for |
|
217 |
details on the error-handling practices.
|
|
1092.1.22
by Robert Collins
update hacking with some test foo |
218 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
219 |
|
220 |
Jargon
|
|
221 |
======
|
|
222 |
||
223 |
revno
|
|
224 |
Integer identifier for a revision on the main line of a branch.
|
|
225 |
Revision 0 is always the null revision; others are 1-based
|
|
226 |
indexes into the branch's revision history. |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
227 |
|
228 |
:: vim: tw=74 ai |