~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

sync with bzr.dev mainline

Show diffs side-by-side

added added

removed removed

Lines of Context:
21
21
  core improvements should be tested closer to the code that is doing the
22
22
  work. Command line level tests should be placed in 'blackbox.py'.
23
23
 
24
 
* Before fixing a bug, write a test case so that it does not regress.
 
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.
25
29
 
26
30
* Exceptions should be defined inside bzrlib.errors, so that we can
27
31
  see the whole tree at a glance.
34
38
* Module names should always be given fully-qualified,
35
39
  i.e. ``bzrlib.hashcache`` not just ``hashcache``.
36
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
 
37
53
Evolving interfaces
38
54
-------------------
39
55
 
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.
 
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.
49
90
 
50
91
Documentation
51
92
=============
53
94
If you change the behaviour of a command, please update its docstring
54
95
in bzrlib/commands.py.  This is displayed by the 'bzr help' command.
55
96
 
 
97
NEWS file
 
98
---------
 
99
 
56
100
If you make a user-visible change, please add a note to the NEWS file.
57
101
The description should be written to make sense to someone who's just
58
102
a user of bzr, not a developer: new functions or classes shouldn't be
60
104
bugs should be listed.  See the existing entries for an idea of what
61
105
should be done.
62
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.
63
121
 
64
122
API documentation
65
123
-----------------
119
177
(not ``x2y`` as occurs in some old code.)
120
178
 
121
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
 
122
202
Writing output
123
203
==============
124
204
 
178
258
Errors and exceptions
179
259
=====================
180
260
 
181
 
[write me]
 
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.
182
265
 
183
266
 
184
267
Jargon
188
271
    Integer identifier for a revision on the main line of a branch.
189
272
    Revision 0 is always the null revision; others are 1-based
190
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