versioned file api audit

[Robert Collins]

On Mon, 2008-03-17 at 20:29 +1100, Martin Pool wrote:
> On 17/03/2008, Robert Collins <robertc at robertcollins.net> wrote:
> > so I just audited the public interface of versioned file. Heres what I
> >  propose we keep/discuss/remove from it.
> >

> >  get_parent_dict (because this is what bzrlib.graph wants; iter_parents
> >  is
> >     better on incremental work)
> 
> I don't see any existing function called get_parent_dict.

get_parent_map - a typo.


> >  iter_lines_added_or_present_in_versions
> >  weave_merge
> >
> >  apis to discuss:
> >  plan_merge (perhaps keep this)
> 
> This is closely related to weave_merge, and so probably should be
> kept.  Maybe it should be a private method and called by rather than
> passed to weave_merge.
> 
> You could say perhaps this will not vary with VF implementation, and
> the vf should instead just give annotation data to something else.

Thats what I'm mulling over :). It's neither hard or easy to keep it; I
haven't done a lot with the text merging stuff, most of my hacking has
been lower or higher in the stack. So moar input please!

> >
> >  apis to remove:
> >  has_version
> 
> Why get rid of this?

Its slow (single version lookup), and it encourages look before you leap
programming; there are extremely few reasons to use this, and a trivial
replacement is
len(collection.get_parent_map([key])) == 1
for the places that really honestly need it. (I think there is one -
has_revision).


> >  iter_parents
> 
> is this not needed?

No - either it or get_parent_map is needed, not both.


> >  clone_text
> 
> "add a new text the same as an old one" is sometime useful; arguably
> not often enough to be a special case; anyhow probably better as an
> option to an add_method

Its already an option to add, clone_text is not used by bzrlib today.

> >  create_empty
> >  get_suffixes
> >  get_text
> >  get_texts
> >  get_lines
> 
> are you really going to have no method to get texts back out?

Bah! I was meaning to choose one :). Lets have two, because we have
substantial performance impact from line splitting/joining, and the
native implementation should be used directly when possible:
get_bytes(versions) -> iterator of (sha1, bytestrings)
get_lines(versions) -> iterator of (sha1, lines_list)

> >  get_ancestry
> >  get_ancestry_with_ghosts
> 
> agree
> 
> >  get_graph
> >  get_graph_with_ghosts
> 
> why get rid of them?  if we plan to have a per-vf graph, this seems
> like the way to expose it.

get_graph returns a dict. the graph is exposed via get_parent_map &
versions. (though I want to rename versions all_keys).

> >  transaction_finished
> 
> agree.
> 
> That would be a nice cleanup!

Thanks :)

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080318/3eae0a04/attachment-0001.pgp