[rfc] [patch] speeding up bzr log with a tree delta iterator

Aaron Bentley aaron.bentley at utoronto.ca
Thu Feb 16 17:17:32 GMT 2006


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

Denys Duchier wrote:
> Thanks for the review,
> 
> none that I can see either. changed.  but I also had to fix get_rev_id: I
> believe the elif should just be an if.

Good catch.

> I believe the thinking behind
> allowing a rev2no keyword argument was that if you have a custom way to iterate
> over revisions, you might also need a custom way to number them...If this keyword argument 
> really bothers you, I
> can take it out, but it does no harm.

I prefer to avoid putting in unused (and presumably untested) code.  You
Ain't Gonna Need It, etc.  If we get it under test, then I would be fine
with keeping it.  Otherwise, it will break and no one will notice 'till
they use it.

But it would be good if we could nuke the semantics for 0 and None, and
use Branch.revision_id_to_revno instead.

> [in_store] was introduced specifically to handle the case requested by jam of a
> lone revid not in the branch's history, and it is used in (builtin.py)cmd_log.

It's shocking that we didn't already have this functionality.  I still
think throwing NoSuchRevision when you don't have to is a bit excessive.
 You'd have to call it something other than in_store, then.  But this is
fine.

The main remaining issue I have is that your latest code still has
public Branch methods that use None to indicate 'no revision specified'.

The main ways I can see fixing this are
1. Define NO_REVISION_SPECIFIED and test for that, instead
2. Make it a private function in log.py

You could also carry through with making 'null:' the only revision-id
for the null revision, but that's a lot of effort for this kind of change.

There are also some integration issues, but I'm happy to fix those myself.

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

iD8DBQFD9LOs0F+nu1YWqI0RAgjhAJ42+x2IPk/Q2Npw2ec2YRmzlyiy5wCfYM4y
EUWH9VDt6hBvAmXsz/E+MNs=
=ZyBA
-----END PGP SIGNATURE-----




More information about the bazaar mailing list