[MERGE] Add simple Storage object
Aaron Bentley
aaron at aaronbentley.com
Tue Jan 29 04:37:13 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Collins wrote:
> I don't see a functional difference between just making everything in
> the base Repository be defined in the primitives you mention, and what
> you present.
I agree there's no functional difference between those two options. I
want to structure it this way because I think it has benefits for people
developing the code
1. It is very clear what a Storage object must implement. It would not
be nearly so clear what a Repository derivative must implement.
2. By grouping new, shiny methods (e.g. get_parent_map) together away
from old, unloved methods (e.g. get_ancestry), it clarifies why there is
overlap between methods-- that one set of methods is the public API and
one set is the implementation API.
3. The implementation API is not a very friendly one, so it does not
make sense to expose it on Repository.
4. The methods on Storage objects are public, but this does not require
all repositories to implement such public methods.
5. It creates a boundary between core and non-core functionality. This
should encourage people to add methods to Repository and not Storage.
This will reduce the growth of the Storage interface.
6. Pack repositories have 81 public methods by my count (see below).
This is large and intimidating, and it does not include
Repository.weave_store or VersionedFile, both of which are arguably part
of the Repository interface. Adding even more methods would not be nice.
7. Smaller interfaces are easier to test, and therefore easier to test
thoroughly.
> But adding another abstraction to Repository, which is already having
> problems - previous cleanups not completed - seems to me to be making
> the api issues there worse, not better.
I am not sure what you're referring to. Could you expand on that?
Even if there are some partially-completed cleanups, I still think that
it is worth it to try and clean up the API. You know, if at first you
don't suceed...
Aaron
P.S. I counted like this:
>>> from bzrlib.repository import Repository
>>> r = Repository.open('.')
>>> len([x for x in dir(r) if callable(getattr(r, x)) and not
x.startswith('_')])
81
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHnq140F+nu1YWqI0RAsUvAJ48qocIab1bd691InvB+bGPASZkfwCdGWR0
1klU8fEFZ4CKKJtvr26ZH0s=
=pob/
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list