[MERGE] HACKING transformed into Bazaar Developer Guide

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:

...

> Latest bundle attached. It includes the fix Alexander requested as well.
> Once again, the net HTML result is attached for those without rst2html
> installed.
> 
> Ian C.
> 

I really like how this is shaping up. I have a few comments, mostly in
regards to reST markup.

When giving specific commands, you want to probably put them in reST
quote blocks, like:

* create a local copy of the main development branch (bzr.dev) by using
  this command::

    bzr branch http://bazaar-vcs.org/bzr/bzr.dev/ bzr.dev

That makes it a lot easier to read in HTML form. (And a bit more obvious
in text form, too).


There are a few places where this happens, (You can generate a bundle
like this).


Talking about stuff like this:
Transports work in URLs. Take note that URLs are by definition only
ASCII - the decision of how to encode a Unicode string into a URL must
be taken at a higher level, typically in the Store. (Note that Stores
also escape filenames which cannot be safely stored on all filesystems,
but this is a different level.)


Doesn't really fit in HACKING. Maybe something more like:
Transports work in URLs. URLs (by definition) are only ASCII - the
decision of how to encode a Unicode string into a URL must be taken at a
higher level.



To be honest, this statement:
We have a commitment to 6 months API stability - any supported symbol in
a release of bzr MUST NOT be altered in any way that would result in ...


Should probably be decremented from MUST NOT, to SHOULD NOT. We have
evolved a bit of pragmatism. Based on the benefit of the new code,
versus the cost of breakage. Or maybe just to "must not". Since that is
a lesser form of MUST :)


In this section:

Module Imports

    * Imports should be done at the top-level of the file, unless there
is a strong reason to have them lazily loaded when a particular function
runs. Import statements have a cost, so try to make sure they don't run
inside hot functions.
    * Module names should always be given fully-qualified, i.e.
bzrlib.hashcache not just hashcache


We really should mention lazy imports, which means that local imports
should go away.

Also that we prefer to do:

from bzrlib import (
    hashcache,
    )
...
hashcache.foo()

to doing

import bzrlib.hashcache
...
bzrlib.hashcache.foo()

or

from bzrlib.hashcache import foo
...
foo()




The 2 reasons I know are:

1) Lazy imports work better if you import modules rather than
functions/classes/objects. There is also less chance that someone is
going to try to import an object from a module it isn't defined in. (foo
is defined in bar.py but imported in baz.py, and someone else does 'from
baz import foo').

2) If you use "bzrlib.module.foo()" there are some race conditions. When
everything is loaded, that generally works. But with lazy importing,
some things that used to be loaded first may not be anymore. Using
"module.foo()" means that you had to "from bzrlib import module", rather
than just "import bzrlib" and getting lucky that "module" had already
been loaded.

It may be enough to just refer users to the Lazy Import section.



"0. Never use a __del__ method without asking Martin/Robert first."

This should probably be "Never use ... without asking a core developer
first."



     0. OK.
     1. Conflicts in merge-like operations, or changes are present in
- -        diff-like operations.
+       diff-like operations.
     2. Unrepresentable diff changes (i.e. binary files that we cannot show
- -        a diff of).
+       a diff of).
     3. An error or exception has occurred.

The old indenting cause reST to format things in a very odd way.



We broadly classify errors as either being either internal or not,
depending on whether user_error is set or not. If we think it's our
fault, we show a backtrace, an invitation to report the bug, and
possibly other details. This is the default for errors that aren't
specifically recognized as being caused by a user error. Otherwise we
show a briefer message, unless -Derror was given.


This is now ``internal_error`` not ``user_error``.



Many errors are defined in bzrlib/errors.py but it's OK for new errors
to be added near the place where they are used.

To date, we haven't followed that policy. AIUI we should only have
errors in errors.py. (We've talked about changing it, but I don't think
we did).


The 'vim' line at the end should be a comment, not a literal block (and
it was formatted wrong anyway):

..
   vim: ...

not
:: vim: ...

(This is probably my fault anyway :)


I'm attaching a diff of the reST cleanups relative to your version.

I didn't do any of the large documentation changes, since I wanted
someone else to agree to them first.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGOLUkJdeBCYSNAAMRApjFAKCXKVVkj1tBzm77EYtR7pSgZVZ5EgCgtvhS
HBjl4IBNqwpFovACjh/H0Gg=
=cvVR
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: minor_hacking_tweaks.diff
Type: text/x-patch
Size: 2819 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070502/0e9cb8ee/attachment-0001.bin