[MERGE] Graph.find_revno()

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> Aaron Bentley wrote:
> | John Arbash Meinel wrote:
> |> This adds a new API to Graph. The idea is to make it easier to determine
> |> a real
> |> 'revno' when all we have is limited information.
> |
> | I'm not sure that this is generally useful.  We usually want to get a
> | revno in the context of a branch, where many revisions don't have
> | integer revnos.
> 
> The biggest thing is that it allows you to give more context than a
> single branch.

I think we are using "context" differently here.

By "context", I mean the perspective from which a revno is determined.
Typically the head revision of a branch.  So I mean something that
should influence the output.

You are using "context" to mean additional information about other
revisions (i.e. their revision numbers).

> Often, we'll have grabbed just a revision from another branch, and then
> we are
> trying to set "self.set_last_revision(revision_id)".

For the specific case of set_last_revision, I agree that your approach
would work, because the head revision is necessarily the same as the
input revision.  And I have no problem with accelerating set_last_revision.

My big concern is that this *looks* like it is general-purpose
functionality, but it is not; because the head revision is not an input
to the function, it can't have the result "no integer revno" -- it will
always give an answer.

Which means it's very easy to accidentally misuse.

> There are also a few places that do 'set_last_revision' style work without
> properly giving the context of the other branch. The idea was to take
> out a lot
> of the if checks, and just provide a nice, well performing function for
> handling
> these cases.

I think it would be ideal to get those cases using
set_last_revision(_info) if possible.

> I admit it isn't strictly useful for things like 'log' where we are
> looking for
> dotted revnos.

The problem is that it's not obviously broken for things like 'log'.  We
could see if different naming (i.e. Graph.left_distance_from_origin,
Graph.revno_as_head, Branch._calculate_head_revno) would help, or we
could change it so it did take a head revision, making it truly
general-purpose.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFINYeg0F+nu1YWqI0RAoh9AJ91ds95l9+NtHlj1fOFKkrivdUMZQCfUt8F
HXhhylIoeztCCZMSg+6Sw2w=
=Hm9y
-----END PGP SIGNATURE-----