[RFC] Splitting bzr-specific functions out of Repository

[Robert Collins]

On Tue, 2008-07-29 at 03:01 +0200, Jelmer Vernooij wrote:
> Am Montag, den 28.07.2008, 09:48 +1000 schrieb Robert Collins:
> > On Sun, 2008-07-27 at 21:05 +0200, Jelmer Vernooij wrote:
> > > Would anybody be opposed to splitting the bzr-specific functions out of
> > > Repository into a BzrRepository class that is a subclass of Repository,
> > > similar to BzrBranch and Branch?
> > I find the bzrbranch/branch split to be quite unsatisfactory. It has
> > lead to some [transient] bugs in fact.
> We don't have any other implementations of Branch in bzrlib that are
> exercised by the tests other than the implementations that derive from
> BzrBranch. That probably hasn't helped; it may be useful to add some
> tests specifically for Branch.

If we have abstract functions that noone can/should override - sure; but
generally if its ok for someone to override a method we should test the
overridden methods.

> > Presumably you're suggesting this to make your life easier :) - what in
> > particular is causing you friction/headaches? (Just having the methods
> > there is at most unclear). Is it needing a more clear definition of what
> > you need to implement, or ???
> Having the methods there is unclear, also means there's no easy way for
> me to determine what I do and what I don't have to implement: other
> parts of Bazaar seem to rely on _ functions even though they are
> underscore functions. 
> 
> This is probably boils down to the discussion we had in London about
> various degrees of accessibility different functions have in Bazaar.

Right. For the list: we were talking about how things (like the CLI)
inside bzr use methods that we do not export to arbitrary users of
bzrlib. Sometimes this is because we are not ready to mark something as
a public method.


> > If we don't want these on the public interface, I'd rather we do the
> > deprecation dance and give them _ names.
> Giving them _ names won't guarantee that other parts of bzrlib won't
>  try to run them.

Neither will taking them off the base class - python isn't c++ :).

> bzr-svn was implemented by trying to run various bzr commands, seeing
> what Repository methods were being executed and then implementing those
> methods. It has happened in the past that _ methods that weren't used by
> any of the builtin commands started being used in some code path
> executed by the builtin commands, breaking bzr-svn. It's also possible
> that I didn't properly exercise all code paths that may use _ functions
> while testing bzr-svn.

I would *dearly* love to have the bzr test suite be capable of running
the full set of bzr-svn tests - interface and branch conformance. This
is really the key to prevent regressions in support IMO.

> It would be much nicer (and work on bzr-git is what brought this up
> again for me) if it was possible to see easily what methods I would have
> to implement.

When I implement a radically different repository format, I lean on the
bzr interface tests very heavily. They are the only exhaustive list that
I know of for checking behaviour.

> Another way of working around this (instead of adding another level of
> subclasses) would be of course be to have the _ and __ functions, _
> meaning "not part of the public API but part of the interface" and __
> meaning completely private. 

The problem is that __foo can't be overridden. So this makes
_overridemeinsubclasses be part of the interface always, even when it
may not be.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080729/c5477cf7/attachment-0001.pgp