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 |
||
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
37 |
Evolving interfaces
|
38 |
-------------------
|
|
39 |
||
40 |
If you change the behaviour of an API in an incompatible way, please
|
|
41 |
be sure to change its name as well. For instance, if I add a keyword
|
|
42 |
parameter to branch.commit - that's fine. On the other hand, if I add |
|
43 |
a keyword parameter to branch.commit which is a *required* transaction |
|
44 |
object, I should rename the api - i.e. to 'branch.commit_transaction'. |
|
45 |
||
46 |
This will prevent users of the old api getting surprising results. |
|
47 |
Instead, they will get an Attribute error as the api is missing, and |
|
48 |
will know to update their code. If in doubt, just ask on #bzr. |
|
49 |
||
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
50 |
Documentation
|
51 |
=============
|
|
52 |
||
53 |
If you change the behaviour of a command, please update its docstring |
|
54 |
in bzrlib/commands.py. This is displayed by the 'bzr help' command. |
|
55 |
||
56 |
If you make a user-visible change, please add a note to the NEWS file. |
|
57 |
The description should be written to make sense to someone who's just |
|
58 |
a user of bzr, not a developer: new functions or classes shouldn't be |
|
59 |
mentioned, but new commands, changes in behaviour or fixed nontrivial |
|
60 |
bugs should be listed. See the existing entries for an idea of what |
|
61 |
should be done. |
|
1098
by Martin Pool
- notes on how output is written |
62 |
|
63 |
||
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
64 |
API documentation |
65 |
-----------------
|
|
66 |
||
67 |
Functions, methods, classes and modules should have docstrings |
|
68 |
describing how they are used. |
|
69 |
||
70 |
The first line of the docstring should be a self-contained sentence. |
|
71 |
||
72 |
For the special case of Command classes, this acts as the user-visible |
|
73 |
documentation shown by the help command. |
|
74 |
||
75 |
The docstrings should be formatted as reStructuredText_ (like this |
|
76 |
document), suitable for processing using the epydoc_ tool into HTML |
|
77 |
documentation. |
|
78 |
||
79 |
.. _reStructuredText: http://docutils.sourceforge.net/rst.html |
|
80 |
.. _epydoc: http://epydoc.sourceforge.net/ |
|
81 |
||
82 |
||
83 |
||
84 |
Coding style |
|
85 |
============
|
|
86 |
||
87 |
Please write PEP-8__ compliant code. |
|
88 |
||
89 |
One often-missed requirement is that the first line of docstrings |
|
90 |
should be a self-contained one-sentence summary. |
|
91 |
||
92 |
__ http://www.python.org/peps/pep-0008.html |
|
93 |
||
94 |
||
95 |
||
96 |
Naming
|
|
97 |
------
|
|
98 |
||
99 |
Functions, methods or members that are in some sense "private" are given |
|
100 |
a leading underscore prefix. This is just a hint that code outside the |
|
101 |
implementation should probably not use that interface. |
|
102 |
||
103 |
We prefer class names to be concatenated capital words (``TestCase``) |
|
104 |
and variables, methods and functions to be lowercase words joined by |
|
105 |
underscores (``revision_id``, ``get_revision``). |
|
106 |
||
107 |
For the purposes of naming some names are treated as single compound |
|
108 |
words: "filename", "revno". |
|
109 |
||
110 |
Consider naming classes as nouns and functions/methods as verbs. |
|
111 |
||
112 |
||
113 |
Standard names |
|
114 |
--------------
|
|
115 |
||
116 |
``revision_id`` not ``rev_id`` or ``revid`` |
|
117 |
||
118 |
Functions that transform one thing to another should be named ``x_to_y`` |
|
119 |
(not ``x2y`` as occurs in some old code.) |
|
120 |
||
1098
by Martin Pool
- notes on how output is written |
121 |
|
122 |
Writing output |
|
123 |
==============
|
|
124 |
||
125 |
(The strategy described here is what we want to get to, but it's not |
|
126 |
consistently followed in the code at the moment.)
|
|
127 |
||
128 |
bzrlib is intended to be a generically reusable library. It shouldn't |
|
129 |
write messages to stdout or stderr, because some programs that use it |
|
130 |
might want to display that information through a GUI or some other |
|
131 |
mechanism. |
|
132 |
||
133 |
We can distinguish two types of output from the library: |
|
134 |
||
135 |
1. Structured data representing the progress or result of an |
|
136 |
operation. For example, for a commit command this will be a list |
|
137 |
of the modified files and the finally committed revision number |
|
138 |
and id. |
|
139 |
||
140 |
These should be exposed either through the return code or by calls |
|
141 |
to a callback parameter. |
|
142 |
||
143 |
A special case of this is progress indicators for long-lived |
|
144 |
operations, where the caller should pass a ProgressBar object. |
|
145 |
||
146 |
2. Unstructured log/debug messages, mostly for the benefit of the |
|
147 |
developers or users trying to debug problems. This should always |
|
148 |
be sent through ``bzrlib.trace`` and Python ``logging``, so that |
|
149 |
it can be redirected by the client. |
|
150 |
||
151 |
The distinction between the two is a bit subjective, but in general if |
|
152 |
there is any chance that a library would want to see something as |
|
153 |
structured data, we should make it so. |
|
154 |
||
155 |
The policy about how output is presented in the text-mode client |
|
156 |
should be only in the command-line tool. |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
157 |
|
1418
by Robert Collins
merge martins latest |
158 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
159 |
Writing tests |
160 |
=============
|
|
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
161 |
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 |
162 |
FOO is the logical thing under test. That file should be placed in the |
163 |
tests subdirectory under the package being tested. |
|
164 |
||
1417.1.1
by Robert Collins
change HACKING test file names to be PEP8 conformant |
165 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py. |
1417.1.2
by Robert Collins
add sample test |
166 |
See bzrlib/selftest/testsampler.py for a template test script. |
1092.1.22
by Robert Collins
update hacking with some test foo |
167 |
|
1393.1.61
by Martin Pool
doc |
168 |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
169 |
Running tests |
170 |
=============
|
|
171 |
Currently, bzr selftest is used to invoke tests. |
|
172 |
You can provide a pattern argument to run a subset. For example, |
|
1393.1.61
by Martin Pool
doc |
173 |
to run just the whitebox tests, run:: |
174 |
||
175 |
bzr selftest -v whitebox |
|
176 |
||
177 |
||
178 |
Errors and exceptions |
|
179 |
=====================
|
|
180 |
||
181 |
[write me] |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
182 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
183 |
|
184 |
Jargon
|
|
185 |
======
|
|
186 |
||
187 |
revno
|
|
188 |
Integer identifier for a revision on the main line of a branch. |
|
189 |
Revision 0 is always the null revision; others are 1-based |
|
190 |
indexes into the branch's revision history. |