[MERGE] Add Branch support to CommitBuilder

Aaron Bentley aaron.bentley at utoronto.ca
Thu Jan 18 00:45:46 GMT 2007

Hash: SHA1

- -1 I don't think you're justified in assuming the local repository and
the remote repository are in the same format.

Jelmer Vernooij wrote:
> On Wed, 2007-01-17 at 11:14 +1100, Martin Pool wrote: 
>> +0.9 from me, just because it adds more redundancy.
> I've attached an update of the bundle. The commit builder class is now
> returned by the repositoryformat. Does this look better?

It looks worse to me.  I think it was in the right place before.

> +        # use local repository to create revision because of speed
> +        if local_branch is not None:
> +            repository = local_branch.repository
> +        else:
> +            repository = self.repository

^^^ this is the bit that earns the veto.  This assumption looks
unjustified.  If you're going to commit locally, do it explicitly, the
way it was before.

> -        return _CommitBuilder(self, parents, config, timestamp, timezone,
> -                              committer, revprops, revision_id)
> +        self._check_revisionid_acceptable(revision_id, self.get_commit_builder)
> +        klass = self._format.get_commit_builder()
> +        return klass(self, parents, config, timestamp, timezone, committer, 
> +                     revprops, revision_id, branch, local_branch)

I don't see why the RepositoryFormat should determine the CommitBuilder
when the Repository controls most of the mechanism actually used for
committing.  For example, the serializer is selected by the Repository,
not the RepositoryFormat, yet miss-matched serializers and
CommitBuilders will often cause commit to fail.

If you want to avoid redefining _get_commit_builder, you can just define
RepositoryFormat._commit_builder_class (as either a method or a class
variable) and construct that in Repository.get_commit_builder()

Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org


More information about the bazaar mailing list