[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