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