~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

  • Committer: Robert Collins
  • Date: 2005-08-25 01:13:32 UTC
  • mto: (974.1.50) (1185.1.10) (1092.3.1)
  • mto: This revision was merged to the branch mainline in revision 1139.
  • Revision ID: robertc@robertcollins.net-20050825011331-6d549d5de7edcec1
two bugfixes to smart_add - do not add paths from nested trees to the parent tree, and do not mutate the user supplied file list

Show diffs side-by-side

added added

removed removed

Lines of Context:
1
1
============================
2
 
Guidelines for modifying bzr
 
2
guidelines for modifying bzr
3
3
============================
4
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
5
* New functionality should have test cases.  Preferably write the
14
6
  test before writing the code.
15
7
 
16
8
  In general, you can test at either the command-line level or the
17
9
  internal API level.  Choose whichever is appropriate: if adding a
18
10
  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'.
 
11
  It is not necessary to do both.
23
12
 
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.
 
13
* Before fixing a bug, write a test case so that it does not regress.
29
14
 
30
15
* Exceptions should be defined inside bzrlib.errors, so that we can
31
16
  see the whole tree at a glance.
35
20
  function runs.  Import statements have a cost, so try to make sure
36
21
  they don't run inside hot functions.
37
22
 
 
23
* Please write PEP-8__ compliant code.  
 
24
 
 
25
  One often-missed requirement is that the first line of docstrings
 
26
  should be a self-contained one-sentence summary.
 
27
 
 
28
__ http://www.python.org/peps/pep-0008.html
 
29
 
38
30
* Module names should always be given fully-qualified,
39
31
  i.e. ``bzrlib.hashcache`` not just ``hashcache``.
40
32
 
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.
 
33
 
90
34
 
91
35
Documentation
92
36
=============
94
38
If you change the behaviour of a command, please update its docstring
95
39
in bzrlib/commands.py.  This is displayed by the 'bzr help' command.
96
40
 
97
 
NEWS file
98
 
---------
99
 
 
100
41
If you make a user-visible change, please add a note to the NEWS file.
101
42
The description should be written to make sense to someone who's just
102
43
a user of bzr, not a developer: new functions or classes shouldn't be
104
45
bugs should be listed.  See the existing entries for an idea of what
105
46
should be done.
106
47
 
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
48
 
201
49
 
202
50
Writing output
235
83
The policy about how output is presented in the text-mode client
236
84
should be only in the command-line tool.
237
85
 
238
 
 
239
86
Writing tests
240
87
=============
241
 
In general tests should be placed in a file named testFOO.py where 
 
88
In general tests should be placed in a file named test_FOO.py where 
242
89
FOO is the logical thing under test. That file should be placed in the
243
90
tests subdirectory under the package being tested.
244
91
 
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
 
 
 
92
For example, tests for merge3 in bzrlib belong in bzrlib/tests/test_merge3.py.
248
93
 
249
94
Running tests
250
95
=============
251
96
Currently, bzr selftest is used to invoke tests.
252
97
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
 
98
to run just the whitebox tests, run bzr selftest --pattern .*whitebox.*
 
99