Foreign branch infrastructure plans

[Martin Pool]

2009/3/16 Robert Collins <robert.collins at canonical.com>:
> On Fri, 2009-03-13 at 15:16 +1100, Martin Pool wrote:
>>
>> It's only a bit related but I'd like to move more towards having clean
>> hierarchies for the main components.  I'm thinking we should have one
>> base which only defines the interface and is purely abstract, and then
>> a subclass that provides 'reasonable defaults' for methods (like
>> "return False" for supports_foo()), or building more complex methods
>> on more primitive ones.  At the moment they're a bit mixed up.
>
> I don't think an ABC per se is useful for us. What we have issues with
> are:
>  - when self-referential convenience methods are defined too low down
>  - when sensible defaults are not sensible :)
>
...
> This may be just what you meant, of course.

That is pretty much what I meant, or at least it's addressing the same
problem.  It would be good to have a consistent approach across the
code but it's probably also best settled case by case than saying
ahead of time that eg all of them must have a pureABC.

If you look at the one case I was dealing with last week,
RemoteBranchFormat.supports_stacking:

1. RemoteRepository is not a subclass of Repository, which to me
indicates that the base Repository may be doing too much stuff or
making too many assumptions for the base class.  (I think this used to
be true for more of the Repository objects but maybe spiv and yourself
have fixed them.)

2. In particular, BranchFormat defaults to supports_stacking() returns
False, which is inherited by concrete subclasses which would have a
different opinion if they had to think about it.

> So I'd like it if we moved all the convenience methods higher up the
> stack, and have things that provide defaults always raise
> NotImplementedError  at the same level that the convenience methods are
> defined.

So then you'd have to specifically hook up the convenience methods,
like hypothetically

  def get_foo(self):
    return self._get_foo_from_wibble()

?

-- 
Martin <http://launchpad.net/~mbp/>