~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to HACKING

- refactor handling of short option names

Show diffs side-by-side

added added

removed removed

Lines of Context:
14
14
  test before writing the code.
15
15
 
16
16
  In general, you can test at either the command-line level or the
17
 
  internal API level.  See Writing Tests below for more detail.
 
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'.
18
23
 
19
 
* Try to practice Test-Driven Development.  before fixing a bug, write a
20
 
  test case so that it does not regress.  Similarly for adding a new
21
 
  feature: write a test case for a small version of the new feature before
22
 
  starting on the code itself.  Check the test fails on the old code, then
23
 
  add the feature or fix and check it passes.
 
24
* Before fixing a bug, write a test case so that it does not regress.
24
25
 
25
26
* Exceptions should be defined inside bzrlib.errors, so that we can
26
27
  see the whole tree at a glance.
33
34
* Module names should always be given fully-qualified,
34
35
  i.e. ``bzrlib.hashcache`` not just ``hashcache``.
35
36
 
36
 
* Commands should return non-zero when they encounter circumstances that
37
 
  the user should really pay attention to - which includes trivial shell
38
 
  pipelines.
39
 
 
40
 
  Recommended values are 
41
 
    0- OK, 
42
 
    1- Conflicts in merge-like operations, or changes are present in
43
 
       diff-like operations. 
44
 
    2- Unrepresentable diff changes (i.e. binary files that we cannot show 
45
 
       a diff of).
46
 
    3- An error or exception has occurred.
47
 
 
48
37
Evolving interfaces
49
38
-------------------
50
39
 
51
 
We have a commitment to 6 months API stability - any supported symbol in a
52
 
release of bzr MUST NOT be altered in any way that would result in
53
 
breaking existing code that uses it. That means that method names,
54
 
parameter ordering, parameter names, variable and attribute names etc must
55
 
not be changed without leaving a 'deprecated forwarder' behind. This even
56
 
applies to modules and classes.
57
 
 
58
 
If you wish to change the behaviour of a supported API in an incompatible
59
 
way, you need to change its name as well. For instance, if I add a optional keyword
60
 
parameter to branch.commit - that's fine. On the other hand, if I add a
61
 
keyword parameter to branch.commit which is a *required* transaction
62
 
object, I should rename the API - i.e. to 'branch.commit_transaction'. 
63
 
 
64
 
When renaming such supported API's, be sure to leave a deprecated_method (or
65
 
_function or ...) behind which forwards to the new API. See the
66
 
bzrlib.symbol_versioning module for decorators that take care of the
67
 
details for you - such as updating the docstring, and issuing a warning
68
 
when the old api is used.
69
 
 
70
 
For unsupported API's, it does not hurt to follow this discipline, but its
71
 
not required. Minimally though, please try to rename things so that
72
 
callers will at least get an AttributeError rather than weird results.
73
 
 
74
 
 
75
 
Standard parameter types
76
 
------------------------
77
 
 
78
 
There are some common requirements in the library: some parameters need to be
79
 
unicode safe, some need byte strings, and so on. At the moment we have
80
 
only codified one specific pattern: Parameters that need to be unicode
81
 
should be check via 'bzrlib.osutils.safe_unicode'. This will coerce the
82
 
input into unicode in a consistent fashion, allowing trivial strings to be
83
 
used for programmer convenience, but not performing unpredictably in the
84
 
presence of different locales.
 
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.
85
49
 
86
50
Documentation
87
51
=============
89
53
If you change the behaviour of a command, please update its docstring
90
54
in bzrlib/commands.py.  This is displayed by the 'bzr help' command.
91
55
 
92
 
NEWS file
93
 
---------
94
 
 
95
56
If you make a user-visible change, please add a note to the NEWS file.
96
57
The description should be written to make sense to someone who's just
97
58
a user of bzr, not a developer: new functions or classes shouldn't be
99
60
bugs should be listed.  See the existing entries for an idea of what
100
61
should be done.
101
62
 
102
 
Within each release, entries in the news file should have the most
103
 
user-visible changes first.  So the order should be approximately:
104
 
 
105
 
 * changes to existing behaviour - the highest priority because the 
106
 
   user's existing knowledge is incorrect
107
 
 * new features - should be brought to their attention
108
 
 * bug fixes - may be of interest if the bug was affecting them, and
109
 
   should include the bug number if any
110
 
 * major documentation changes
111
 
 * changes to internal interfaces
112
 
 
113
 
People who made significant contributions to each change are listed in
114
 
parenthesis.  This can include reporting bugs (particularly with good
115
 
details or reproduction recipes), submitting patches, etc.
116
63
 
117
64
API documentation
118
65
-----------------
172
119
(not ``x2y`` as occurs in some old code.)
173
120
 
174
121
 
175
 
Destructors
176
 
-----------
177
 
 
178
 
Python destructors (``__del__``) work differently to those of other
179
 
languages.  In particular, bear in mind that destructors may be called
180
 
immediately when the object apparently becomes unreferenced, or at some
181
 
later time, or possibly never at all.  Therefore we have restrictions on
182
 
what can be done inside them.
183
 
 
184
 
 0. Never use a __del__ method without asking Martin/Robert first.
185
 
 
186
 
 1. Never rely on a ``__del__`` method running.  If there is code that
187
 
    must run, do it from a ``finally`` block instead.
188
 
 
189
 
 2. Never ``import`` from inside a ``__del__`` method, or you may crash the
190
 
    interpreter!!
191
 
 
192
 
 3. In some places we raise a warning from the destructor if the object
193
 
    has not been cleaned up or closed.  This is considered OK: the warning
194
 
    may not catch every case but it's still useful sometimes.
195
 
 
196
 
 
197
122
Writing output
198
123
==============
199
124
 
233
158
 
234
159
Writing tests
235
160
=============
236
 
In general tests should be placed in a file named test_FOO.py where 
 
161
In general tests should be placed in a file named testFOO.py where 
237
162
FOO is the logical thing under test. That file should be placed in the
238
163
tests subdirectory under the package being tested.
239
164
 
240
 
For example, tests for merge3 in bzrlib belong in bzrlib/tests/test_merge3.py.
241
 
See bzrlib/selftest/test_sampler.py for a template test script.
242
 
 
243
 
Tests can be written for the UI or for individual areas of the library.
244
 
Choose whichever is appropriate: if adding a new command, or a new command 
245
 
option, then you should be writing a UI test.  If you are both adding UI
246
 
functionality and library functionality, you will want to write tests for 
247
 
both the UI and the core behaviours.  We call UI tests 'blackbox' tests
248
 
and they are found in bzrlib/tests/blackbox/*.py. 
249
 
 
250
 
When writing blackbox tests please honour the following conventions:
251
 
 
252
 
 1. Place the tests for the command 'name' in
253
 
    bzrlib/tests/blackbox/test_name.py. This makes it easy for developers
254
 
    to locate the test script for a faulty command.
255
 
 
256
 
 2. Use the 'self.run_bzr("name")' utility function to invoke the command
257
 
    rather than running bzr in a subprocess or invoking the
258
 
    cmd_object.run() method directly. This is a lot faster than
259
 
    subprocesses and generates the same logging output as running it in a
260
 
    subprocess (which invoking the method directly does not).
261
 
 
262
 
 3. Only test the one command in a single test script. Use the bzrlib 
263
 
    library when setting up tests and when evaluating the side-effects of
264
 
    the command. We do this so that the library api has continual pressure
265
 
    on it to be as functional as the command line in a simple manner, and
266
 
    to isolate knock-on effects throughout the blackbox test suite when a
267
 
    command changes it name or signature. Ideally only the tests for a
268
 
    given command are affected when a given command is changed.
 
165
For example, tests for merge3 in bzrlib belong in bzrlib/tests/testmerge3.py.
 
166
See bzrlib/selftest/testsampler.py for a template test script.
 
167
 
269
168
 
270
169
Running tests
271
170
=============
272
171
Currently, bzr selftest is used to invoke tests.
273
172
You can provide a pattern argument to run a subset. For example, 
274
 
to run just the blackbox tests, run::
 
173
to run just the whitebox tests, run::
275
174
 
276
 
  ./bzr selftest -v blackbox
 
175
  bzr selftest -v whitebox
277
176
 
278
177
 
279
178
Errors and exceptions
280
179
=====================
281
180
 
282
 
Errors are handled through Python exceptions.  They can represent user
283
 
errors, environmental errors or program bugs.  Sometimes we can't be sure
284
 
at the time it's raised which case applies.  See bzrlib/errors.py for 
285
 
details on the error-handling practices.
 
181
[write me]
286
182
 
287
183
 
288
184
Jargon
292
188
    Integer identifier for a revision on the main line of a branch.
293
189
    Revision 0 is always the null revision; others are 1-based
294
190
    indexes into the branch's revision history.
295
 
 
296
 
 
297
 
Transport
298
 
=========
299
 
 
300
 
The ``Transport`` layer handles access to local or remote directories.
301
 
Each Transport object acts like a logical connection to a particular
302
 
directory, and it allows various operations on files within it.  You can
303
 
*clone* a transport to get a new Transport connected to a subdirectory or
304
 
parent directory.
305
 
 
306
 
Transports are not used for access to the working tree.  At present
307
 
working trees are always local and they are accessed through the regular
308
 
Python file io mechanisms.
309
 
 
310
 
filenames vs URLs
311
 
-----------------
312
 
 
313
 
Transports work in URLs.  Take note that URLs are by definition only
314
 
ASCII - the decision of how to encode a Unicode string into a URL must be
315
 
taken at a higher level, typically in the Store.  (Note that Stores also
316
 
escape filenames which cannot be safely stored on all filesystems, but
317
 
this is a different level.)
318
 
 
319
 
The main reason for this is that it's not possible to safely roundtrip a
320
 
URL into Unicode and then back into the same URL.  The URL standard
321
 
gives a way to represent non-ASCII bytes in ASCII (as %-escapes), but
322
 
doesn't say how those bytes represent non-ASCII characters.  (They're not
323
 
guaranteed to be UTF-8 -- that is common but doesn't happen everywhere.)
324
 
 
325
 
For example if the user enters the url ``http://example/%e0`` there's no
326
 
way to tell whether that character represents "latin small letter a with
327
 
grave" in iso-8859-1, or "latin small letter r with acute" in iso-8859-2
328
 
or malformed UTF-8.  So we can't convert their URL to Unicode reliably.
329
 
 
330
 
Equally problematic if we're given a url-like string containing non-ascii
331
 
characters (such as the accented a) we can't be sure how to convert that
332
 
to the correct URL, because we don't know what encoding the server expects
333
 
for those characters.  (Although this is not totally reliable we might still
334
 
accept these and assume they should be put into UTF-8.)
335
 
 
336
 
A similar edge case is that the url ``http://foo/sweet%2Fsour" contains
337
 
one directory component whose name is "sweet/sour".  The escaped slash is
338
 
not a directory separator.  If we try to convert URLs to regular Unicode
339
 
paths this information will be lost.
340
 
 
341
 
This implies that Transports must natively deal with URLs; for simplicity
342
 
they *only* deal with URLs and conversion of other strings to URLs is done
343
 
elsewhere.  Information they return, such as from ``list_dir``, is also in
344
 
the form of URL components.
345
 
 
346
 
 
347
 
Merge/review process
348
 
====================
349
 
 
350
 
If you'd like to propose a change, please post to the
351
 
bazaar-ng@lists.canonical.com list with a patch, bzr changeset, or link to a
352
 
branch.  Please put '[patch]' in the subject so we can pick them out, and
353
 
include some text explaining the change.  Remember to put an update to the NEWS
354
 
file in your diff, if it makes any changes visible to users or plugin
355
 
developers.  Please include a diff against mainline if you're giving a link to
356
 
a branch.
357
 
 
358
 
Please indicate if you think the code is ready to merge, or if it's just a
359
 
draft or for discussion.  If you want comments from many developers rather than
360
 
to be merged, you can put '[rfc]' in the subject lines.
361
 
 
362
 
Anyone is welcome to review code.  There are broadly three gates for
363
 
code to get in:
364
 
 
365
 
 * Doesn't reduce test coverage: if it adds new methods or commands,
366
 
   there should be tests for them.  There is a good test framework
367
 
   and plenty of examples to crib from, but if you are having trouble
368
 
   working out how to test something feel free to post a draft patch
369
 
   and ask for help.
370
 
 
371
 
 * Doesn't reduce design clarity, such as by entangling objects
372
 
   we're trying to separate.  This is mostly something the more
373
 
   experienced reviewers need to help check.
374
 
 
375
 
 * Improves bugs, features, speed, or code simplicity.
376
 
 
377
 
Code that goes in should pass all three.
378
 
 
379
 
If you read a patch please reply and say so.  We can use a numeric scale
380
 
of -1, -0, +0, +1, meaning respectively "really don't want it in current
381
 
form", "somewhat uncomfortable", "ok with me", and "please put it in".
382
 
Anyone can "vote".   (It's not really voting, just a terse expression.)
383
 
 
384
 
If something gets say two +1 votes from core reviewers, and no
385
 
vetos, then it's OK to come in.  Any of the core developers can bring it
386
 
into their integration branch, which I'll merge regularly.  (If you do
387
 
so, please reply and say so.)
388
 
 
389
 
 
390
 
:: vim:tw=74:ai