[RFC][PATCH 1/4] Speed improvement in fetch/clone: file_involved( ) function

[Goffredo Baroncelli]

On Thursday 15 December 2005 05:27, you (Robert Collins) wrote:
> On Sat, 2005-12-10 at 19:15 +0100, Goffredo Baroncelli wrote:
> > This patch add the function file involved( )
> > 
> > === modified file 'bzrlib/branch.py'
> > --- bzrlib/branch.py
> > +++ bzrlib/branch.py
> > @@ -1090,6 +1090,55 @@
> >          self.revision_store.add(StringIO(gpg_strategy.sign(plaintext)),
> >                                  revision_id, "sig")
> > 
> > +    def file_involved(self, arg1=None, arg2=None):
> 
> This seems like something generally useful to "Branch" - so the function
> should be defined on Branch as a 'raise NotImplemented' stub, and then
> the BzrBranch implementation in the BzrBranch class.

Ok
[...]

> Lastly, I note the 'if isinstance' calls - that makes apis hard to use -
> can we please not do this unless we really need to.
> 
> I'd rather see one of the following variations:
> def file_involved(self, revisions=None, require_ancestor=None,
> require_descendant=None):
>    ...
>    revisions is a set of revisions to consider or None 
>    for no constraint.
>  
>    require_ancestor: a revision_id to require as an ancestor in
>    the found revisions.
> 
>    require_descendant: a revision_id that must be a descendant
>    in the found revisions.
> 
>    These three constraints are combined with AND clauses - all
>    specified constraints must be met for a revision to be returned.
>    """
> 
> This is more flexible than the arg1, arg2 approach, and IMO clearer and
> easier to use and understand when someone sees this code elsewhere.

I prefer an api like:

file_involved_set( )		# extracts the file involved on the basis of a set of
				# revs

file_involved_upto( )		# extracts the file involved up to a revision

file_involved_revisions( )	# extract the file_involved between two revision

Moreover I think that instead file_involved( ), a better name is weave_involved( ):
what do you think ?

Goffredo

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreijack @ inwind.it>
Key fingerprint = CE3C 7E01 6782 30A3 5B87  87C0 BB86 505C 6B2A CFF9