[MERGE] Let find_previous_heads take a repository.

[John Arbash Meinel]

Johan Rydberg wrote:
> Aaron Bentley <aaron.bentley at utoronto.ca> writes:
> 
>>> Robert Collins <robertc at robertcollins.net> writes:
>>> Robert and I discussed this a bit on IRC.  Here's a new patch to shoot
>>> down.  We both dislike the fact that find_previous_heads use either a
>>> repository, or a passed repository_file, but because of how the
>>> upgrade code works it is a must, I'm afraid.
>> It looks as though VersionedFile is meant to be a type of
>> RepositoryFile.  Shouldn't it subclass it?
> 
> Sounds reasonable.

I realize it isn't required because of DuckTyping, but probably to help
developer's mental model it would help.

> 
>> I'm not sure about RepositoryFile as part of our API.  It seems to me
>> that VersionedFile is a reasonable level of abstraction, and that it can
>> describe all our past and present ways of referring to versions of files
>> reasonably well.
>>
>> When RepositoryFile is fleshed out more, how will it differ from
>> VersionedFile?
> 
> RepositoryFile will be a more slimmed interface than VersionedFile.
> It will for example not be possible to insert new history via the
> RepositoryFile interface.  History is added using either the fetcher
> or the CommitBuilder.  
> 
> VersionedFile extends RepositoryFile, so there is no need to alter the
> existing VersionedFile implementations (knits and weaves).  
> 
> We need a per-file interface for retrieving history information out of
> the repository.  This is what RepositoryFile is meant for.

Why can't you just use VersionedFile with some methods NotImplemented?

> 
> ~j

...

> 
> My Python skills is somewhat, but shouldn't this work?
> 
>       already_present = bool([head for head in heads if version_id in
>                                   head_ancestors[head]])

Yes, they should work, but it still suffers from 'what the hell are you
trying to say' syndrome. :)

Aaron's version also has the property of recognizing you don't care how
many there are, or what they are, you just want to know if there is one.
 Which means you can skip out of your loop as soon as you find one.
Rather than probing all of them, building up a list, and then throwing
it all away.

Between being more obvious, and maybe performing better, I would prefer
Aaron's method.


> Thanks for your comments,
> 
> ~j

Why do you prefer returning a stripped down object, rather than having
the call directly on Repository. Your RepositoryFile interface takes no
constructor, and saves no state. So it is obviously never returned. I
suppose it defines a potential API. But as it stands the class
definition is next to useless.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060619/52b47bd0/attachment.pgp