74
73
Bazaar Development in a Nutshell
75
74
================================
77
.. was from bazaar-vcs.org/BzrGivingBack
79
One of the fun things about working on a version control system like Bazaar is
80
that the users have a high level of proficiency in contributing back into
81
the tool. Consider the following very brief introduction to contributing back
82
to Bazaar. More detailed instructions are in the following sections.
87
First, get a local copy of the development mainline (See `Why make a local
93
$ bzr branch http://bazaar-vcs.org/bzr/bzr.dev/ bzr.dev
95
Now make your own branch::
97
$ bzr branch bzr.dev 123456-my-bugfix
99
This will give you a branch called "123456-my-bugfix" that you can work on
100
and commit in. Here, you can study the code, make a fix or a new feature.
101
Feel free to commit early and often (after all, it's your branch!).
103
Documentation improvements are an easy place to get started giving back to the
104
Bazaar project. The documentation is in the `doc/` subdirectory of the Bazaar
107
When you are done, make sure that you commit your last set of changes as well!
108
Once you are happy with your changes, ask for them to be merged, as described
111
Making a Merge Proposal
112
-----------------------
114
The Bazaar developers use Launchpad to further enable a truly distributed
115
style of development. Anyone can propose a branch for merging into the Bazaar
116
trunk. To start this process, you need to push your branch to Launchpad. To
117
do this, you will need a Launchpad account and user name, e.g.
118
`your_lp_username`. You can push your branch to Launchpad directly from
121
$ bzr push lp:~your_lp_username/bzr/meaningful_name_here
123
After you have pushed your branch, you will need to propose it for merging to
124
the Bazaar trunk. Go to
125
<https://launchpad.net/your_lp_username/bzr/meaningful_name_here> and choose
126
"Propose for merging into another branch". Select "~bzr/bzr/trunk" to hand
127
your changes off to the Bazaar developers for review and merging.
129
Alternatively, after pushing you can use the ``lp-propose`` command to
130
create the merge proposal.
132
Using a meaningful name for your branch will help you and the reviewer(s)
133
better track the submission. Use a very succint description of your submission
134
and prefix it with bug number if needed (lp:~mbp/bzr/484558-merge-directory
135
for example). Alternatively, you can suffix with the bug number
136
(lp:~jameinel/bzr/export-file-511987).
142
Please put a "cover letter" on your merge request explaining:
144
* the reason **why** you're making this change
146
* **how** this change achieves this purpose
148
* anything else you may have fixed in passing
150
* anything significant that you thought of doing, such as a more
151
extensive fix or a different approach, but didn't or couldn't do now
153
A good cover letter makes reviewers' lives easier because they can decide
154
from the letter whether they agree with the purpose and approach, and then
155
assess whether the patch actually does what the cover letter says.
156
Explaining any "drive-by fixes" or roads not taken may also avoid queries
157
from the reviewer. All in all this should give faster and better reviews.
158
Sometimes writing the cover letter helps the submitter realize something
159
else they need to do. The size of the cover letter should be proportional
160
to the size and complexity of the patch.
163
Why make a local copy of bzr.dev?
164
---------------------------------
166
Making a local mirror of bzr.dev is not strictly necessary, but it means
168
- You can use that copy of bzr.dev as your main bzr executable, and keep it
169
up-to-date using ``bzr pull``.
170
- Certain operations are faster, and can be done when offline. For example:
173
- ``bzr diff -r ancestor:...``
176
- When it's time to create your next branch, it's more convenient. When you
177
have further contributions to make, you should do them in their own branch::
180
$ bzr branch bzr.dev additional_fixes
181
$ cd additional_fixes # hack, hack, hack
76
Looking for a 10 minute introduction to submitting a change?
77
See http://bazaar-vcs.org/BzrGivingBack.
79
TODO: Merge that Wiki page into this document.
185
82
Understanding the Development Process
279
178
Holds documentation on a whole range of things on Bazaar from the
280
179
origination of ideas within the project to information on Bazaar
281
180
features and use cases. Within this directory there is a subdirectory
282
for each translation into a human language. All the documentation
181
for each translation into a human language. All the documentation
283
182
is in the ReStructuredText markup language.
286
Documentation specifically targeted at Bazaar and plugin developers.
185
Documentation specifically targetted at Bazaar and plugin developers.
287
186
(Including this document.)
291
Automatically-generated API reference information is available at
292
<http://starship.python.net/crew/mwh/bzrlibapi/>.
294
See also the `Bazaar Architectural Overview
295
<http://doc.bazaar-vcs.org/developers/overview.html>`_.
190
Automatically-generated API reference information is available at
191
<http://starship.python.net/crew/mwh/bzrlibapi/>.
193
See also the `Bazaar Architectural Overview <../../developers/overview.html>`_.
196
The Code Review Process
197
#######################
199
All code changes coming in to Bazaar are reviewed by someone else.
200
Normally changes by core contributors are reviewed by one other core
201
developer, and changes from other people are reviewed by two core
202
developers. Use intelligent discretion if the patch is trivial.
204
Good reviews do take time. They also regularly require a solid
205
understanding of the overall code base. In practice, this means a small
206
number of people often have a large review burden - with knowledge comes
207
responsibility. No one like their merge requests sitting in a queue going
208
nowhere, so reviewing sooner rather than later is strongly encouraged.
212
Sending patches for review
213
==========================
215
If you'd like to propose a change, please post to the
216
bazaar@lists.canonical.com list with a bundle, patch, or link to a
217
branch. Put ``[PATCH]`` or ``[MERGE]`` in the subject so Bundle Buggy
218
can pick it out, and explain the change in the email message text.
219
Remember to update the NEWS file as part of your change if it makes any
220
changes visible to users or plugin developers. Please include a diff
221
against mainline if you're giving a link to a branch.
223
You can generate a merge request like this::
225
bzr send -o bug-1234.patch
227
A ``.patch`` extension is recommended instead of .bundle as many mail clients
228
will send the latter as a binary file.
230
``bzr send`` can also send mail directly if you prefer; see the help.
232
Please do **NOT** put [PATCH] or [MERGE] in the subject line if you don't
233
want it to be merged. If you want comments from developers rather than
234
to be merged, you can put ``[RFC]`` in the subject line.
236
If this change addresses a bug, please put the bug number in the subject
237
line too, in the form ``[#1]`` so that Bundle Buggy can recognize it.
239
If the change is intended for a particular release mark that in the
240
subject too, e.g. ``[1.6]``.
246
Please put a "cover letter" on your merge request explaining:
248
* the reason **why** you're making this change
250
* **how** this change achieves this purpose
252
* anything else you may have fixed in passing
254
* anything significant that you thought of doing, such as a more
255
extensive fix or a different approach, but didn't or couldn't do now
257
A good cover letter makes reviewers' lives easier because they can decide
258
from the letter whether they agree with the purpose and approach, and then
259
assess whether the patch actually does what the cover letter says.
260
Explaining any "drive-by fixes" or roads not taken may also avoid queries
261
from the reviewer. All in all this should give faster and better reviews.
262
Sometimes writing the cover letter helps the submitter realize something
263
else they need to do. The size of the cover letter should be proportional
264
to the size and complexity of the patch.
267
Reviewing proposed changes
268
==========================
270
Anyone is welcome to review code, and reply to the thread with their
273
The simplest way to review a proposed change is to just read the patch on
274
the list or in Bundle Buggy. For more complex changes it may be useful
275
to make a new working tree or branch from trunk, and merge the proposed
276
change into it, so you can experiment with the code or look at a wider
279
There are three main requirements for code to get in:
281
* Doesn't reduce test coverage: if it adds new methods or commands,
282
there should be tests for them. There is a good test framework
283
and plenty of examples to crib from, but if you are having trouble
284
working out how to test something feel free to post a draft patch
287
* Doesn't reduce design clarity, such as by entangling objects
288
we're trying to separate. This is mostly something the more
289
experienced reviewers need to help check.
291
* Improves bugs, features, speed, or code simplicity.
293
Code that goes in should not degrade any of these aspects. Patches are
294
welcome that only cleanup the code without changing the external
295
behaviour. The core developers take care to keep the code quality high
296
and understandable while recognising that perfect is sometimes the enemy
299
It is easy for reviews to make people notice other things which should be
300
fixed but those things should not hold up the original fix being accepted.
301
New things can easily be recorded in the Bug Tracker instead.
303
It's normally much easier to review several smaller patches than one large
304
one. You might want to use ``bzr-loom`` to maintain threads of related
305
work, or submit a preparatory patch that will make your "real" change
309
Checklist for reviewers
310
=======================
312
* Do you understand what the code's doing and why?
314
* Will it perform reasonably for large inputs, both in memory size and
315
run time? Are there some scenarios where performance should be
318
* Is it tested, and are the tests at the right level? Are there both
319
blackbox (command-line level) and API-oriented tests?
321
* If this change will be visible to end users or API users, is it
322
appropriately documented in NEWS?
324
* Does it meet the coding standards below?
326
* If it changes the user-visible behaviour, does it update the help
327
strings and user documentation?
329
* If it adds a new major concept or standard practice, does it update the
330
developer documentation?
332
* (your ideas here...)
335
Bundle Buggy and review outcomes
336
================================
338
Anyone can "vote" on the mailing list by expressing an opinion. Core
339
developers can also vote using Bundle Buggy. Here are the voting codes and
342
:approve: Reviewer wants this submission merged.
343
:tweak: Reviewer wants this submission merged with small changes. (No
345
:abstain: Reviewer does not intend to vote on this patch.
346
:resubmit: Please make changes and resubmit for review.
347
:reject: Reviewer doesn't want this kind of change merged.
348
:comment: Not really a vote. Reviewer just wants to comment, for now.
350
If a change gets two approvals from core reviewers, and no rejections,
351
then it's OK to come in. Any of the core developers can bring it into the
352
bzr.dev trunk and backport it to maintenance branches if required. The
353
Release Manager will merge the change into the branch for a pending
354
release, if any. As a guideline, core developers usually merge their own
355
changes and volunteer to merge other contributions if they were the second
356
reviewer to agree to a change.
358
To track the progress of proposed changes, use Bundle Buggy. See
359
http://bundlebuggy.aaronbentley.com/help for a link to all the
360
outstanding merge requests together with an explanation of the columns.
361
Bundle Buggy will also mail you a link to track just your change.
363
Coding Style Guidelines
364
#######################
369
``hasattr`` should not be used because it swallows exceptions including
370
``KeyboardInterrupt``. Instead, say something like ::
372
if getattr(thing, 'name', None) is None
378
Please write PEP-8__ compliant code.
380
__ http://www.python.org/peps/pep-0008.html
382
One often-missed requirement is that the first line of docstrings
383
should be a self-contained one-sentence summary.
385
We use 4 space indents for blocks, and never use tab characters. (In vim,
388
No trailing white space is allowed.
390
Unix style newlines (LF) are used.
392
Each file must have a newline at the end of it.
394
Lines should be no more than 79 characters if at all possible.
395
Lines that continue a long statement may be indented in either of
398
within the parenthesis or other character that opens the block, e.g.::
404
or indented by four spaces::
410
The first is considered clearer by some people; however it can be a bit
411
harder to maintain (e.g. when the method name changes), and it does not
412
work well if the relevant parenthesis is already far to the right. Avoid
415
self.legbone.kneebone.shinbone.toebone.shake_it(one,
421
self.legbone.kneebone.shinbone.toebone.shake_it(one,
427
self.legbone.kneebone.shinbone.toebone.shake_it(
430
For long lists, we like to add a trailing comma and put the closing
431
character on the following line. This makes it easier to add new items in
434
from bzrlib.goo import (
440
There should be spaces between function paramaters, but not between the
441
keyword name and the value::
443
call(1, 3, cheese=quark)
447
;(defface my-invalid-face
448
; '((t (:background "Red" :underline t)))
449
; "Face used to highlight invalid constructs or other uglyties"
452
(defun my-python-mode-hook ()
453
;; setup preferred indentation style.
454
(setq fill-column 79)
455
(setq indent-tabs-mode nil) ; no tabs, never, I will not repeat
456
; (font-lock-add-keywords 'python-mode
457
; '(("^\\s *\t" . 'my-invalid-face) ; Leading tabs
458
; ("[ \t]+$" . 'my-invalid-face) ; Trailing spaces
459
; ("^[ \t]+$" . 'my-invalid-face)); Spaces only
463
(add-hook 'python-mode-hook 'my-python-mode-hook)
465
The lines beginning with ';' are comments. They can be activated
466
if one want to have a strong notice of some tab/space usage
473
* Imports should be done at the top-level of the file, unless there is
474
a strong reason to have them lazily loaded when a particular
475
function runs. Import statements have a cost, so try to make sure
476
they don't run inside hot functions.
478
* Module names should always be given fully-qualified,
479
i.e. ``bzrlib.hashcache`` not just ``hashcache``.
485
Functions, methods or members that are "private" to bzrlib are given
486
a leading underscore prefix. Names without a leading underscore are
487
public not just across modules but to programmers using bzrlib as an
488
API. As a consequence, a leading underscore is appropriate for names
489
exposed across modules but that are not to be exposed to bzrlib API
492
We prefer class names to be concatenated capital words (``TestCase``)
493
and variables, methods and functions to be lowercase words joined by
494
underscores (``revision_id``, ``get_revision``).
496
For the purposes of naming some names are treated as single compound
497
words: "filename", "revno".
499
Consider naming classes as nouns and functions/methods as verbs.
501
Try to avoid using abbreviations in names, because there can be
502
inconsistency if other people use the full name.
508
``revision_id`` not ``rev_id`` or ``revid``
510
Functions that transform one thing to another should be named ``x_to_y``
511
(not ``x2y`` as occurs in some old code.)
517
Python destructors (``__del__``) work differently to those of other
518
languages. In particular, bear in mind that destructors may be called
519
immediately when the object apparently becomes unreferenced, or at some
520
later time, or possibly never at all. Therefore we have restrictions on
521
what can be done inside them.
523
0. If you think you need to use a ``__del__`` method ask another
524
developer for alternatives. If you do need to use one, explain
527
1. Never rely on a ``__del__`` method running. If there is code that
528
must run, do it from a ``finally`` block instead.
530
2. Never ``import`` from inside a ``__del__`` method, or you may crash the
533
3. In some places we raise a warning from the destructor if the object
534
has not been cleaned up or closed. This is considered OK: the warning
535
may not catch every case but it's still useful sometimes.
541
In some places we have variables which point to callables that construct
542
new instances. That is to say, they can be used a lot like class objects,
543
but they shouldn't be *named* like classes:
545
> I think that things named FooBar should create instances of FooBar when
546
> called. Its plain confusing for them to do otherwise. When we have
547
> something that is going to be used as a class - that is, checked for via
548
> isinstance or other such idioms, them I would call it foo_class, so that
549
> it is clear that a callable is not sufficient. If it is only used as a
550
> factory, then yes, foo_factory is what I would use.
556
Several places in Bazaar use (or will use) a registry, which is a
557
mapping from names to objects or classes. The registry allows for
558
loading in registered code only when it's needed, and keeping
559
associated information such as a help string or description.
562
InterObject and multiple dispatch
563
=================================
565
The ``InterObject`` provides for two-way `multiple dispatch`__: matching
566
up for example a source and destination repository to find the right way
567
to transfer data between them.
569
.. __: http://en.wikipedia.org/wiki/Multiple_dispatch
571
There is a subclass ``InterObject`` classes for each type of object that is
572
dispatched this way, e.g. ``InterRepository``. Calling ``.get()`` on this
573
class will return an ``InterObject`` instance providing the best match for
574
those parameters, and this instance then has methods for operations
577
inter = InterRepository.get(source_repo, target_repo)
578
inter.fetch(revision_id)
580
``InterRepository`` also acts as a registry-like object for its
581
subclasses, and they can be added through ``.register_optimizer``. The
582
right one to run is selected by asking each class, in reverse order of
583
registration, whether it ``.is_compatible`` with the relevant objects.
588
To make startup time faster, we use the ``bzrlib.lazy_import`` module to
589
delay importing modules until they are actually used. ``lazy_import`` uses
590
the same syntax as regular python imports. So to import a few modules in a
593
from bzrlib.lazy_import import lazy_import
594
lazy_import(globals(), """
603
revision as _mod_revision,
605
import bzrlib.transport
609
At this point, all of these exist as a ``ImportReplacer`` object, ready to
610
be imported once a member is accessed. Also, when importing a module into
611
the local namespace, which is likely to clash with variable names, it is
612
recommended to prefix it as ``_mod_<module>``. This makes it clearer that
613
the variable is a module, and these object should be hidden anyway, since
614
they shouldn't be imported into other namespaces.
616
While it is possible for ``lazy_import()`` to import members of a module
617
when using the ``from module import member`` syntax, it is recommended to
618
only use that syntax to load sub modules ``from module import submodule``.
619
This is because variables and classes can frequently be used without
620
needing a sub-member for example::
622
lazy_import(globals(), """
623
from module import MyClass
627
return isinstance(x, MyClass)
629
This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer``
630
object, rather than the real class.
632
It also is incorrect to assign ``ImportReplacer`` objects to other variables.
633
Because the replacer only knows about the original name, it is unable to
634
replace other variables. The ``ImportReplacer`` class will raise an
635
``IllegalUseOfScopeReplacer`` exception if it can figure out that this
636
happened. But it requires accessing a member more than once from the new
637
variable, so some bugs are not detected right away.
643
The null revision is the ancestor of all revisions. Its revno is 0, its
644
revision-id is ``null:``, and its tree is the empty tree. When referring
645
to the null revision, please use ``bzrlib.revision.NULL_REVISION``. Old
646
code sometimes uses ``None`` for the null revision, but this practice is
650
Object string representations
651
=============================
653
Python prints objects using their ``__repr__`` method when they are
654
written to logs, exception tracebacks, or the debugger. We want
655
objects to have useful representations to help in determining what went
658
If you add a new class you should generally add a ``__repr__`` method
659
unless there is an adequate method in a parent class. There should be a
662
Representations should typically look like Python constructor syntax, but
663
they don't need to include every value in the object and they don't need
664
to be able to actually execute. They're to be read by humans, not
665
machines. Don't hardcode the classname in the format, so that we get the
666
correct value if the method is inherited by a subclass. If you're
667
printing attributes of the object, including strings, you should normally
668
use ``%r`` syntax (to call their repr in turn).
670
Try to avoid the representation becoming more than one or two lines long.
671
(But balance this against including useful information, and simplicity of
674
Because repr methods are often called when something has already gone
675
wrong, they should be written somewhat more defensively than most code.
676
The object may be half-initialized or in some other way in an illegal
677
state. The repr method shouldn't raise an exception, or it may hide the
678
(probably more useful) underlying exception.
683
return '%s(%r)' % (self.__class__.__name__,
690
A bare ``except`` statement will catch all exceptions, including ones that
691
really should terminate the program such as ``MemoryError`` and
692
``KeyboardInterrupt``. They should rarely be used unless the exception is
693
later re-raised. Even then, think about whether catching just
694
``Exception`` (which excludes system errors in Python2.5 and later) would
701
All code should be exercised by the test suite. See `Guide to Testing
702
Bazaar <../../developers/testing.html>`_ for detailed information about writing tests.
added
removed
doc/developers/HACKING.txt
