Code review comment for lp:~jelmer/bzr/uncommit-remove-tags

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

On 8/24/2011 12:10 PM, Jelmer Vernooij wrote:
> On 08/24/2011 11:46 AM, John A Meinel wrote:
>> On 8/24/2011 7:30 AM, Martin Pool wrote:
>>> Review: Approve That's good.
>>>
>>> [tweak] It's a bit safer to add new keyword parameters at the
>>> end.
>>>
>>> [tweak] Please mention it in the user guide under uncommit.
>> I'm a bit concerned that we do a graph search to find what tags
>> to remove. I guess we've only been doing a linear search to
>> resolve what revisions to display?
>>
> Yep. We could potentially avoid redoing the get_parent_map calls
> for the revisions on the mainline, but at the cost of more code
> complexity.
>
> Cheers,
>
> Jelmer
>

Right, if we only know about mainline, it isn't worth worrying about.
As long as things are well cached, etc.

I'd worry a bit about performance, but uncommit should usually be
local enough in scope it should be ok. Though I wish our
find_unique_ancestors code didn't have some really bad behaviors
(landing bzr-2.1 into bzr-2.4 causes a lot of graph searching, though
some of that is just because we are 'skipping over' 2 years of history.)

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

iEYEARECAAYFAk5U1J8ACgkQJdeBCYSNAAP8LwCeJYZt9z82Vc0tH6bYzSy6kStN
eI0An09oAtrCdzasSV4UVpFNZ1GlhogZ
=qfkU
-----END PGP SIGNATURE-----