← Back to branch summary

~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

Merge from integration.

Show diffs side-by-side

added added

removed removed

Lines of Context:
HACKING
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
 
37
 
* Commands should return Non-Zero when they encounter circumstances that
 
41
* Commands should return non-zero when they encounter circumstances that
38
42
  the user should really pay attention to - which includes trivial shell
39
43
  pipelines.
40
44
 
49
53
Evolving interfaces
50
54
-------------------
51
55
 
52
 
If you change the behaviour of an API in an incompatible way, please
53
 
be sure to change its name as well. For instance, if I add a keyword
54
 
parameter to branch.commit - that's fine. On the other hand, if I add
55
 
a keyword parameter to branch.commit which is a *required* transaction
56
 
object, I should rename the API - i.e. to 'branch.commit_transaction'.
57
 
 
58
 
This will prevent users of the old API getting surprising results. 
59
 
Instead, they will get an Attribute error as the API is missing, and
60
 
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.
61
90
 
62
91
Documentation
63
92
=============
243
272
    Revision 0 is always the null revision; others are 1-based
244
273
    indexes into the branch's revision history.
245
274
 
246
 
:: vim: tw=74 ai
 
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