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 |
||
1185.34.1
by Jelmer Vernooij
Fix a couple of typo's |
41 |
Recommended values are
|
1476
by Robert Collins
Merge now has a retcode of 1 when conflicts occur. (Robert Collins) |
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).
|
|
1185.34.1
by Jelmer Vernooij
Fix a couple of typo's |
47 |
3- An error or exception has occurred.
|
1476
by Robert Collins
Merge now has a retcode of 1 when conflicts occur. (Robert Collins) |
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 |
|
1185.34.1
by Jelmer Vernooij
Fix a couple of typo's |
56 |
object, I should rename the API - i.e. to 'branch.commit_transaction'. |
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
57 |
|
1185.34.1
by Jelmer Vernooij
Fix a couple of typo's |
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 |
|
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
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 |
||
1185.33.2
by Martin Pool
How to maintain the NEWS file |
68 |
NEWS file |
69 |
---------
|
|
70 |
||
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
71 |
If you make a user-visible change, please add a note to the NEWS file. |
72 |
The description should be written to make sense to someone who's just |
|
73 |
a user of bzr, not a developer: new functions or classes shouldn't be |
|
74 |
mentioned, but new commands, changes in behaviour or fixed nontrivial |
|
75 |
bugs should be listed. See the existing entries for an idea of what |
|
76 |
should be done. |
|
1098
by Martin Pool
- notes on how output is written |
77 |
|
1185.33.2
by Martin Pool
How to maintain the NEWS file |
78 |
Within each release, entries in the news file should have the most |
79 |
user-visible changes first. So the order should be approximately: |
|
80 |
||
81 |
* changes to existing behaviour - the highest priority because the |
|
82 |
user's existing knowledge is incorrect |
|
83 |
* new features - should be brought to their attention
|
|
84 |
* bug fixes - may be of interest if the bug was affecting them, and
|
|
85 |
should include the bug number if any
|
|
86 |
* major documentation changes
|
|
87 |
* changes to internal interfaces
|
|
88 |
||
89 |
People who made significant contributions to each change are listed in
|
|
90 |
parenthesis. This can include reporting bugs (particularly with good
|
|
91 |
details or reproduction recipes), submitting patches, etc.
|
|
1098
by Martin Pool
- notes on how output is written |
92 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
93 |
API documentation
|
94 |
-----------------
|
|
95 |
||
96 |
Functions, methods, classes and modules should have docstrings
|
|
97 |
describing how they are used.
|
|
98 |
||
99 |
The first line of the docstring should be a self-contained sentence.
|
|
100 |
||
101 |
For the special case of Command classes, this acts as the user-visible
|
|
102 |
documentation shown by the help command.
|
|
103 |
||
104 |
The docstrings should be formatted as reStructuredText_ (like this
|
|
105 |
document), suitable for processing using the epydoc_ tool into HTML
|
|
106 |
documentation.
|
|
107 |
||
108 |
.. _reStructuredText: http://docutils.sourceforge.net/rst.html
|
|
109 |
.. _epydoc: http://epydoc.sourceforge.net/
|
|
110 |
||
111 |
||
112 |
||
113 |
Coding style
|
|
114 |
============
|
|
115 |
||
116 |
Please write PEP-8__ compliant code.
|
|
117 |
||
118 |
One often-missed requirement is that the first line of docstrings
|
|
119 |
should be a self-contained one-sentence summary.
|
|
120 |
||
121 |
__ http://www.python.org/peps/pep-0008.html
|
|
122 |
||
123 |
||
124 |
||
125 |
Naming
|
|
126 |
------
|
|
127 |
||
128 |
Functions, methods or members that are in some sense "private" are given
|
|
129 |
a leading underscore prefix. This is just a hint that code outside the
|
|
130 |
implementation should probably not use that interface.
|
|
131 |
||
132 |
We prefer class names to be concatenated capital words (``TestCase``)
|
|
133 |
and variables, methods and functions to be lowercase words joined by
|
|
134 |
underscores (``revision_id``, ``get_revision``).
|
|
135 |
||
136 |
For the purposes of naming some names are treated as single compound
|
|
137 |
words: "filename", "revno".
|
|
138 |
||
139 |
Consider naming classes as nouns and functions/methods as verbs.
|
|
140 |
||
141 |
||
142 |
Standard names
|
|
143 |
--------------
|
|
144 |
||
145 |
``revision_id`` not ``rev_id`` or ``revid``
|
|
146 |
||
147 |
Functions that transform one thing to another should be named ``x_to_y``
|
|
148 |
(not ``x2y`` as occurs in some old code.)
|
|
149 |
||
1098
by Martin Pool
- notes on how output is written |
150 |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
151 |
Destructors
|
152 |
-----------
|
|
153 |
||
1185.16.150
by Martin Pool
Improved description of python exception policies |
154 |
Python destructors (``__del__``) work differently to those of other
|
155 |
languages. In particular, bear in mind that destructors may be called
|
|
156 |
immediately when the object apparently becomes unreferenced, or at some
|
|
157 |
later time, or possibly never at all. Therefore we have restrictions on
|
|
158 |
what can be done inside them.
|
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
159 |
|
160 |
0. Never use a __del__ method without asking Martin/Robert first.
|
|
161 |
||
162 |
1. Never rely on a ``__del__`` method running. If there is code that
|
|
163 |
must run, do it from a ``finally`` block instead.
|
|
164 |
||
165 |
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
|
|
166 |
interpreter!!
|
|
167 |
||
168 |
3. In some places we raise a warning from the destructor if the object
|
|
169 |
has not been cleaned up or closed. This is considered OK: the warning
|
|
170 |
may not catch every case but it's still useful sometimes. |
|
171 |
||
172 |
||
1098
by Martin Pool
- notes on how output is written |
173 |
Writing output |
174 |
==============
|
|
175 |
||
176 |
(The strategy described here is what we want to get to, but it's not |
|
177 |
consistently followed in the code at the moment.)
|
|
178 |
||
179 |
bzrlib is intended to be a generically reusable library. It shouldn't |
|
180 |
write messages to stdout or stderr, because some programs that use it |
|
181 |
might want to display that information through a GUI or some other |
|
182 |
mechanism. |
|
183 |
||
184 |
We can distinguish two types of output from the library: |
|
185 |
||
186 |
1. Structured data representing the progress or result of an |
|
187 |
operation. For example, for a commit command this will be a list |
|
188 |
of the modified files and the finally committed revision number |
|
189 |
and id. |
|
190 |
||
191 |
These should be exposed either through the return code or by calls |
|
192 |
to a callback parameter. |
|
193 |
||
194 |
A special case of this is progress indicators for long-lived |
|
195 |
operations, where the caller should pass a ProgressBar object. |
|
196 |
||
197 |
2. Unstructured log/debug messages, mostly for the benefit of the |
|
198 |
developers or users trying to debug problems. This should always |
|
199 |
be sent through ``bzrlib.trace`` and Python ``logging``, so that |
|
200 |
it can be redirected by the client. |
|
201 |
||
202 |
The distinction between the two is a bit subjective, but in general if |
|
203 |
there is any chance that a library would want to see something as |
|
204 |
structured data, we should make it so. |
|
205 |
||
206 |
The policy about how output is presented in the text-mode client |
|
207 |
should be only in the command-line tool. |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
208 |
|
1418
by Robert Collins
merge martins latest |
209 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
210 |
Writing tests |
211 |
=============
|
|
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
212 |
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 |
213 |
FOO is the logical thing under test. That file should be placed in the |
214 |
tests subdirectory under the package being tested. |
|
215 |
||
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
216 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py. |
1417.1.2
by Robert Collins
add sample test |
217 |
See bzrlib/selftest/testsampler.py for a template test script. |
1092.1.22
by Robert Collins
update hacking with some test foo |
218 |
|
1393.1.61
by Martin Pool
doc |
219 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
220 |
Running tests |
221 |
=============
|
|
222 |
Currently, bzr selftest is used to invoke tests. |
|
223 |
You can provide a pattern argument to run a subset. For example, |
|
1393.1.61
by Martin Pool
doc |
224 |
to run just the whitebox tests, run:: |
225 |
||
226 |
bzr selftest -v whitebox |
|
227 |
||
228 |
||
229 |
Errors and exceptions |
|
230 |
=====================
|
|
231 |
||
1185.16.61
by mbp at sourcefrog
- start introducing hct error classes |
232 |
Errors are handled through Python exceptions. They can represent user |
233 |
errors, environmental errors or program bugs. Sometimes we can't be sure |
|
234 |
at the time it's raised which case applies. See bzrlib/errors.py for |
|
235 |
details on the error-handling practices. |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
236 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
237 |
|
238 |
Jargon
|
|
239 |
======
|
|
240 |
||
241 |
revno
|
|
242 |
Integer identifier for a revision on the main line of a branch. |
|
243 |
Revision 0 is always the null revision; others are 1-based |
|
244 |
indexes into the branch's revision history. |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
245 |
|
246 |
:: vim: tw=74 ai
|