[MERGE] More deprecations...

[Robert Collins]

On Thu, 2008-03-27 at 13:35 -0400, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > On Thu, 2008-03-27 at 16:45 +1100, Andrew Bennetts wrote:
> > Firstly, _make_breadth_first_searcher is not private; its just not
> > public. We took the stance some time ago that within bzrlib _ is
> > advisory for *external users*, not for internal.
> 
> Look, I don't remember us taking that stance, and it seems overly
> Microsoftian to be using sekret APIs.  If we need the API, someone else
> will, too.

Clearly not everything is either stable or internal to a class. We dont
have any way to annotate this today; despite long threads about it. I'm
really not interested in another: *my* understanding is that anything
within bzrlib can use any other thing unless the method explicitly says
in its docstring 'do not use this'. I think this is unsatisfactory and
leads to confusion - and look, it is. This is I think the third time in
as many months that this has come up. (I'm not annoyed with you here
Aaron; just expressing frustration that we are here suffering the
compromise of a known-incomplete solution).

The current stance is expressed in HACKING:
---
Functions, methods or members that are "private" to bzrlib are given
a leading underscore prefix.  Names without a leading underscore are
public not just across modules but to programmers using bzrlib as an
API. As a consequence, a leading underscore is appropriate for names
exposed across modules but that are not to be exposed to bzrlib API
programmers.
---
(I read 'bzrlib API programmers' as 'folk writing plugins or using
bzrlib in their own projects').

> > My overall goal here is to get to a VersionedFiles api which is tunable
> > to perform well, and doesn't have cruft on it.
> 
> That's admirable, but you still haven't responded to my last email about
> the namespace issue.

We seemed to be at an impasse; I don't think you had more to say, and
neither did I.

> > To do this regardless of conversion approach, we have to stop using the
> > old methods that are on 'VersionedFile'. We can do this a number of
> > ways: we can leave a thunk permanently in place, we can use external
> > functions, we can do basic macro expansion at each current site, or we
> > can fix the code up properly.
> 
> Aren't you leaving out the option of using an API that delivers
> effectively the same data?
>
> AIUI dict(Graph.iter_ancestry(head)) is essentially the same as
> get_revision_graph(head).

not at all its still basically macro expanding what used to be a single
function call. Note too that 'dict(a_graph.iter_ancestry(head))' is
about the same as the code I used up to the map() call; the filtering
beyond that to remove NULL_REVISION and ghost parents (both of which are
included by the graphs returned from repository.get_graph) is needed for
the behaviour of the places I put it in, and is what makes the
duplication quite visible.

Also, the idiom I used is faster than iter_ancestry, because it does
less calls into generators (and we should stop doing full history!). 

That said I'm happy to change these to seed the parent_map with
dict(..iter_ancestry...) instead; but I don't think it makes the smell
go away.

-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/20080328/68f1fe8d/attachment.pgp