~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

  • Committer: Martin Pool
  • Date: 2005-04-26 05:51:17 UTC
  • Revision ID: mbp@sourcefrog.net-20050426055116-e4bede04e549e4f6
- update doc upload scripts

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