[MERGE] Graph.find_revno()

[John Arbash Meinel]

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

Aaron Bentley wrote:
| 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

I'm happy to rename it. It isn't called "find_dotted_revno", and the pure
"revno" is a static integer that is fixed regardless of branch. I'll try to come
up with a better name.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEUEARECAAYFAkg1mgoACgkQJdeBCYSNAANo5QCfdp0KIxM+Kt+ROP78uT7SdGje
cxAAlR6igv8l+FSnhySTpWf79dUfTWA=
=JucS
-----END PGP SIGNATURE-----