[MERGE] Review feedback on dirstate update_basis_via_delta logic.

John Arbash Meinel john at arbash-meinel.com
Wed Oct 24 21:23:27 BST 2007


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

Robert Collins wrote:
> On Wed, 2007-10-24 at 13:36 -0400, John Arbash Meinel wrote:
>> John Arbash Meinel has voted tweak.
>> Status is now: Conditionally approved
>> Comment:
>> I wish you had included a delta of the change since your previous 
>> submission. Since it would make reviewing your incremental changes a lot 
>> easier.
> 
> Well, diff -r -2..-1 ;). It is a bundle after all.

Sure, and that is what I did. But I had to pull it into a temp branch first,
and do all that work.
Versus just seeing it in Thunderbird.

> 
>> It seems like we should have two simple runs of 'bzr check', one without 
>> a working tree, and one with a working tree. Just to catch any typos in 
>> the with WT version. (I might also argue that we need one with the first 
>> commit [no basis], and one with at least one commit.)
> 
> I'll add one with a wt and with a commit.
> 
>> I feel a little weird using tree._iter_changes() as we have been trying 
>> hard to get that special cased/optimized. I realize right now it is 
>> fine, but potentially it could check tree.get_revision_id() and realize 
>> it can use the fast-path code. While we really just want a very simple 
>> (and thus very unlikely to be incorrect) code path checking.
> 
> This is a possible concern. Looking at it, I think we can change the
> order of tree_basis and repo_basis so the repository revision tree
> object is the one asked to diff. And the fast path we care about is
> tree->basis, not basis->arbitrary version.

...

> Martin, what do you think of this patch? Its a big performance boost,
> but given that it's in fairly core code and potentially still has
> unknown bugs (I'm not aware of any), it is late in the 0.92 cycle for
> it.
> 
> We could merge it now, or we could merge it without the change to
> commit.py to enable the fast path, or we could just wait for 0.93 to
> open.
> 
> -Rob

The importance of the change does seem to warrant some bzr.dev time.
Considering the fallout from the 0.15 release. (Feisty *still* only has 0.15
available in the default repos)

Though I would like to get it in right away. Ideally before all-hands so we can
point to the fact that it is in bzr.dev.

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

iD8DBQFHH6m/JdeBCYSNAAMRAibJAJ9qsM3cB1pl7utJGMFSB033k3xg8gCdEkNM
GCkOXpKo9hjbSgxiAZe/fWg=
=FKx5
-----END PGP SIGNATURE-----



More information about the bazaar mailing list