[MERGE] RevisionSpec.as_revision_id()

[Aaron Bentley]

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

John Arbash Meinel wrote:
> +            #skip the last revision in the list
> +            assert iterator.next() == start_revision
> 
> ^- This should not be done in an assert, as it will be skipped when we run
> python -O.
> 
> So I would recommend:
> 
> next_rev = iterator.next()
> assert next_rev == start_revision

Good catch.  Man, do I ever wish we had mandatory asserts.

> +
> ^- Here, if history is not None, you check its length, but then ignore
> it. The
> point of passing in history is that the calling function already had to
> compute it.

I didn't mean to leave that out, and I've fixed it.

Still, if you get to a point where you *have* the entire history,
something is really wrong.  And if you *do* get the entire history,
shouldn't it be cached in the _partial_history_cache?

> I also didn't see any code for "cache_revision_history" to actually set
> any data
> in _partial_revision_history_cache. In theory it should set
> self._revision_history_cache = self._partial_revision_history_cache =
> history

Branch.revision_history populates that cache.  I haven't changed that.

> And I'm *guessing* that the actual performance difference you are seeing
> is that
> Robert's patch for improved iter_history... was finally merged into
> bzr.dev, but
> isn't in my patch.

No, I had merged bzr.dev into your patch, so the only difference was my
code changes.  But that kind of performance difference is within the
margin of error.

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

iD8DBQFH9CH50F+nu1YWqI0RArNZAKCJIYc9RDD8vF/Avtr6KFgekmQzSACfcCGf
xLE5WjC7A8t7FAiXeSmm1IY=
=l+jD
-----END PGP SIGNATURE-----