[MERGE] Add ForeignBranch

Aaron Bentley aaron at aaronbentley.com
Sun Dec 28 00:24:53 GMT 2008


Jelmer Vernooij wrote:
> Hi Aaron,
> 
> Am Samstag, den 27.12.2008, 18:37 -0500 schrieb Aaron Bentley:
>> Jelmer Vernooij wrote:
>>> The attached patch adds a simple ForeignBranch class, as it is used at
>>> the moment as base class for the Branch classes in bzr-svn and bzr-git.
>> Hi, I don't want to be a blocker on this, but could you explain it a bit
>> more?
>>
>> What's the "mapping" ivar?
> 
> The mapping ivar is a instance of "bzrlib.foreign.VcsMapping", or a
> class derived from it. It is the mapping that will be used to generate
> bzr revisions from foreign VCS metadata. In the case of Subversion,
> there have historically been four different subclasses of VcsMapping,
> each added in a different major release of bzr-svn, and each improving
> the way the mapping between bzr and svn is done.

Okay, great.  That should be in a docstring somewhere.

>> Why are you defining dpush here?  Are you going to test whether branches
>> are ForeignBranches and then call dpush if they are?
> dpush is a separate bzr subcommand that is similar to "bzr push"

I know what an implementation would do.  I'm trying to understand the
function of ForeignBranch.  I'm not sure if it's a helper class, or an
interface, or some combination.

ForeignBranch seems way too short to be anything useful.  I'm surprised
that ForeignBranch.dpush is there, yet not implemented.  I would have
thought it would at least use the "template method" pattern, and call
private methods which would then be supplied by subclasses.

If we have ForeignBranch.dpush, shouldn't we also have tests for that
interface, i.e. for all implementations of ForeignBranch?

Aaron



More information about the bazaar mailing list