~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

update TODO

Show diffs side-by-side

added added

removed removed

Lines of Context:
 
1
============================
 
2
Guidelines for modifying bzr
 
3
============================
 
4
 
 
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
 
 
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().
 
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'.
 
23
 
 
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.
 
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
 
 
41
* Commands should return non-zero when they encounter circumstances that
 
42
  the user should really pay attention to - which includes trivial shell
 
43
  pipelines.
 
44
 
 
45
  Recommended values are 
 
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).
 
51
    3- An error or exception has occurred.
 
52
 
 
53
Evolving interfaces
 
54
-------------------
 
55
 
 
56
We have a commitment to 6 months API stability - any supported symbol in a
 
57
release of bzr MUST NOT be altered in any way that would result in
 
58
breaking existing code that uses it. That means that method names,
 
59
parameter ordering, parameter names, variable and attribute names etc must
 
60
not be changed without leaving a 'deprecated forwarder' behind. This even
 
61
applies to modules and classes.
 
62
 
 
63
If you wish to change the behaviour of a supported API in an incompatible
 
64
way, you need to change its name as well. For instance, if I add a optional keyword
 
65
parameter to branch.commit - that's fine. On the other hand, if I add a
 
66
keyword parameter to branch.commit which is a *required* transaction
 
67
object, I should rename the API - i.e. to 'branch.commit_transaction'. 
 
68
 
 
69
When renaming such supported API's, be sure to leave a deprecated_method (or
 
70
_function or ...) behind which forwards to the new API. See the
 
71
bzrlib.symbol_versioning module for decorators that take care of the
 
72
details for you - such as updating the docstring, and issuing a warning
 
73
when the old api is used.
 
74
 
 
75
For unsupported API's, it does not hurt to follow this discipline, but its
 
76
not required. Minimally though, please try to rename things so that
 
77
callers will at least get an AttributeError rather than weird results.
 
78
 
 
79
 
 
80
Standard parameter types
 
81
------------------------
 
82
 
 
83
There are some common requirements in the library: some parameters need to be
 
84
unicode safe, some need byte strings, and so on. At the moment we have
 
85
only codified one specific pattern: Parameters that need to be unicode
 
86
should be check via 'bzrlib.osutils.safe_unicode'. This will coerce the
 
87
input into unicode in a consistent fashion, allowing trivial strings to be
 
88
used for programmer convenience, but not performing unpredictably in the
 
89
presence of different locales.
 
90
 
 
91
Documentation
 
92
=============
 
93
 
 
94
If you change the behaviour of a command, please update its docstring
 
95
in bzrlib/commands.py.  This is displayed by the 'bzr help' command.
 
96
 
 
97
NEWS file
 
98
---------
 
99
 
 
100
If you make a user-visible change, please add a note to the NEWS file.
 
101
The description should be written to make sense to someone who's just
 
102
a user of bzr, not a developer: new functions or classes shouldn't be
 
103
mentioned, but new commands, changes in behaviour or fixed nontrivial
 
104
bugs should be listed.  See the existing entries for an idea of what
 
105
should be done.
 
106
 
 
107
Within each release, entries in the news file should have the most
 
108
user-visible changes first.  So the order should be approximately:
 
109
 
 
110
 * changes to existing behaviour - the highest priority because the 
 
111
   user's existing knowledge is incorrect
 
112
 * new features - should be brought to their attention
 
113
 * bug fixes - may be of interest if the bug was affecting them, and
 
114
   should include the bug number if any
 
115
 * major documentation changes
 
116
 * changes to internal interfaces
 
117
 
 
118
People who made significant contributions to each change are listed in
 
119
parenthesis.  This can include reporting bugs (particularly with good
 
120
details or reproduction recipes), submitting patches, etc.
 
121
 
 
122
API documentation
 
123
-----------------
 
124
 
 
125
Functions, methods, classes and modules should have docstrings
 
126
describing how they are used. 
 
127
 
 
128
The first line of the docstring should be a self-contained sentence.
 
129
 
 
130
For the special case of Command classes, this acts as the user-visible
 
131
documentation shown by the help command.
 
132
 
 
133
The docstrings should be formatted as reStructuredText_ (like this
 
134
document), suitable for processing using the epydoc_ tool into HTML
 
135
documentation.
 
136
 
 
137
.. _reStructuredText: http://docutils.sourceforge.net/rst.html
 
138
.. _epydoc: http://epydoc.sourceforge.net/
 
139
 
 
140
 
 
141
 
 
142
Coding style
 
143
============
 
144
 
 
145
Please write PEP-8__ compliant code.  
 
146
 
 
147
One often-missed requirement is that the first line of docstrings
 
148
should be a self-contained one-sentence summary.
 
149
 
 
150
__ http://www.python.org/peps/pep-0008.html
 
151
 
 
152
 
 
153
 
 
154
Naming
 
155
------
 
156
 
 
157
Functions, methods or members that are in some sense "private" are given
 
158
a leading underscore prefix.  This is just a hint that code outside the
 
159
implementation should probably not use that interface.
 
160
 
 
161
We prefer class names to be concatenated capital words (``TestCase``)
 
162
and variables, methods and functions to be lowercase words joined by
 
163
underscores (``revision_id``, ``get_revision``).
 
164
 
 
165
For the purposes of naming some names are treated as single compound
 
166
words: "filename", "revno".
 
167
 
 
168
Consider naming classes as nouns and functions/methods as verbs.
 
169
 
 
170
 
 
171
Standard names
 
172
--------------
 
173
 
 
174
``revision_id`` not ``rev_id`` or ``revid``
 
175
 
 
176
Functions that transform one thing to another should be named ``x_to_y``
 
177
(not ``x2y`` as occurs in some old code.)
 
178
 
 
179
 
 
180
Destructors
 
181
-----------
 
182
 
 
183
Python destructors (``__del__``) work differently to those of other
 
184
languages.  In particular, bear in mind that destructors may be called
 
185
immediately when the object apparently becomes unreferenced, or at some
 
186
later time, or possibly never at all.  Therefore we have restrictions on
 
187
what can be done inside them.
 
188
 
 
189
 0. Never use a __del__ method without asking Martin/Robert first.
 
190
 
 
191
 1. Never rely on a ``__del__`` method running.  If there is code that
 
192
    must run, do it from a ``finally`` block instead.
 
193
 
 
194
 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
 
195
    interpreter!!
 
196
 
 
197
 3. In some places we raise a warning from the destructor if the object
 
198
    has not been cleaned up or closed.  This is considered OK: the warning
 
199
    may not catch every case but it's still useful sometimes.
 
200
 
 
201
 
 
202
Writing output
 
203
==============
 
204
 
 
205
(The strategy described here is what we want to get to, but it's not
 
206
consistently followed in the code at the moment.)
 
207
 
 
208
bzrlib is intended to be a generically reusable library.  It shouldn't
 
209
write messages to stdout or stderr, because some programs that use it
 
210
might want to display that information through a GUI or some other
 
211
mechanism.
 
212
 
 
213
We can distinguish two types of output from the library:
 
214
 
 
215
 1. Structured data representing the progress or result of an
 
216
    operation.  For example, for a commit command this will be a list
 
217
    of the modified files and the finally committed revision number
 
218
    and id.
 
219
 
 
220
    These should be exposed either through the return code or by calls
 
221
    to a callback parameter.
 
222
 
 
223
    A special case of this is progress indicators for long-lived
 
224
    operations, where the caller should pass a ProgressBar object.
 
225
 
 
226
 2. Unstructured log/debug messages, mostly for the benefit of the
 
227
    developers or users trying to debug problems.  This should always
 
228
    be sent through ``bzrlib.trace`` and Python ``logging``, so that
 
229
    it can be redirected by the client.
 
230
 
 
231
The distinction between the two is a bit subjective, but in general if
 
232
there is any chance that a library would want to see something as
 
233
structured data, we should make it so.
 
234
 
 
235
The policy about how output is presented in the text-mode client
 
236
should be only in the command-line tool.
 
237
 
 
238
 
 
239
Writing tests
 
240
=============
 
241
In general tests should be placed in a file named testFOO.py where 
 
242
FOO is the logical thing under test. That file should be placed in the
 
243
tests subdirectory under the package being tested.
 
244
 
 
245
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py.
 
246
See bzrlib/selftest/testsampler.py for a template test script.
 
247
 
 
248
 
 
249
Running tests
 
250
=============
 
251
Currently, bzr selftest is used to invoke tests.
 
252
You can provide a pattern argument to run a subset. For example, 
 
253
to run just the whitebox tests, run::
 
254
 
 
255
  bzr selftest -v whitebox
 
256
 
 
257
 
 
258
Errors and exceptions
 
259
=====================
 
260
 
 
261
Errors are handled through Python exceptions.  They can represent user
 
262
errors, environmental errors or program bugs.  Sometimes we can't be sure
 
263
at the time it's raised which case applies.  See bzrlib/errors.py for 
 
264
details on the error-handling practices.
 
265
 
 
266
 
 
267
Jargon
 
268
======
 
269
 
 
270
revno
 
271
    Integer identifier for a revision on the main line of a branch.
 
272
    Revision 0 is always the null revision; others are 1-based
 
273
    indexes into the branch's revision history.
 
274
 
 
275
 
 
276
Merge/review process
 
277
====================
 
278
 
 
279
If you'd like to propose a change, please post to the
 
280
bazaar-ng@lists.canonical.com list with a patch, bzr changeset, or link to a
 
281
branch.  Please put '[patch]' in the subject so we can pick them out, and
 
282
include some text explaining the change.  Remember to put an update to the NEWS
 
283
file in your diff, if it makes any changes visible to users or plugin
 
284
developers.  Please include a diff against mainline if you're giving a link to
 
285
a branch.
 
286
 
 
287
Please indicate if you think the code is ready to merge, or if it's just a
 
288
draft or for discussion.  If you want comments from many developers rather than
 
289
to be merged, you can put '[rfc]' in the subject lines.
 
290
 
 
291
Anyone is welcome to review code.  There are broadly three gates for
 
292
code to get in:
 
293
 
 
294
 * Doesn't reduce test coverage: if it adds new methods or commands,
 
295
   there should be tests for them.  There is a good test framework
 
296
   and plenty of examples to crib from, but if you are having trouble
 
297
   working out how to test something feel free to post a draft patch
 
298
   and ask for help.
 
299
 
 
300
 * Doesn't reduce design clarity, such as by entangling objects
 
301
   we're trying to separate.  This is mostly something the more
 
302
   experienced reviewers need to help check.
 
303
 
 
304
 * Improves bugs, features, speed, or code simplicity.
 
305
 
 
306
Code that goes in should pass all three.
 
307
 
 
308
If you read a patch please reply and say so.  We can use a numeric scale
 
309
of -1, -0, +0, +1, meaning respectively "really don't want it in current
 
310
form", "somewhat uncomfortable", "ok with me", and "please put it in".
 
311
Anyone can "vote".   (It's not really voting, just a terse expression.)
 
312
 
 
313
If something gets say two +1 votes from core reviewers, and no
 
314
vetos, then it's OK to come in.  Any of the core developers can bring it
 
315
into their integration branch, which I'll merge regularly.  (If you do
 
316
so, please reply and say so.)
 
317
 
 
318
 
 
319
:: vim:tw=74:ai