[MERGE] (robertc) Knit repo format tidyups from the packs branch. (Robert Collins)

Ian Clatworthy ian.clatworthy at internode.on.net
Thu Sep 27 04:17:26 BST 2007


Robert Collins wrote:
> On Wed, 2007-09-26 at 12:01 +1000, Ian Clatworthy wrote:

>>> -        dirs = ['revision-store', 'knits']
>>> +        dirs = ['knits']
>> So RepositoryFormatKnit.initialize will no longer create a
>> revision-store directory. Is this no longer required? Is it done
>> elsewhere now? This is the change I can't approve because I don't know
>> enough about whether it is safe or not.
> 
> Its a bug fix. Weave based repositories used a revision-store, knits
> never have.

Cool.

bb: tweak

>>> +    repository_class = KnitRepository
>>> +
>> You've added this change to RepositoryFormatKnit1 when I think it ought
>> to go higher in the inheritance hierarchy, namely in
>> RepositoryFormatKnit. I say that because RepositoryFormatKnit.open has
>> been changed to use that attribute now (instead of the hard-coded
>> KnitRepository) so it ought to exist there.
> 
> RepositoryFormatKnit is now an abstract base class, so no, it should not
> be present at that level, only in concrete classes.

I disagree with this. In most OO languages, an abstract base class still
needs to compile - it needs to reference methods and attributes that
exist, even if those things are themselves abstract. I can't see why
that rule ought to be broken just because we're using Python as the
implementation language.

If you want to set repository_class to None in the abstract class and
leave the "repository_class = KnitRepository" assignment in the concrete
class, I'm ok with that.

Ian C.



More information about the bazaar mailing list