copy to get this too you, regular one will follow when I get real net again :)

[Martin Pool]

On Fri, Jul 11, 2008 at 7:37 PM, Robert Collins
<robert.collins at gmail.com> wrote:

>> This potentially causes a problem because I think now the base class
>> version will be active, and that does not use the kvf index but rather
>> reads each revision one by one, which may be slow.  I have verified
>> that method is on the instance but haven't profiled it.  It should be
>> fairly straightforward to fix.
>
> Yes; thats an oversight for sure - we want to just return
> keys = [(revid,) for revid in revision_ids]
> return self.revisions.get_parent_map(keys)
>
> (Modulo returning NULL_REVISION if johns goal of pushing that deeper
> hasn't been done yet, and handling None).

Right, that is what it should do - but there are already other similar
implementations so I was going to work out if one of them should be
used somehow instead.

btw, it's a bit risky that there are several methods called
get_parent_map, some of which take keys and some revision ids.  I
realize the transition is underway.  But Python being what it is, if
you call one the wrong way you're likely to get the wrong result
rather than an error.  Maybe the next time around it would be better
to call it eg get_parent_map_keys() or something.

>
>
> Do you want this merged? Seems intrusive...(e.g. may hang PQM :))
> +            import pdb; pdb.post_mortem(err[2])

No, I did a diff to the wt rather than -1.

I do actually want a better version of that merged as it's very
useful.  Maybe activated through selftest --debug, not unconditionally
of course.

> I would rather we keep this test but invert the sense: it should work
> rather than failing :).
>
> -    def test_error_mismatched_format(self):
> -        # this first implementation uses the internal pack list from
> each
> -        # repository and thus needs the same format in both sides.
> -        base = self.make_repository('base', format='pack-0.92')
> -        repo = self.make_repository('repo', format=self.get_format())
> -        self.assertRaises(errors.IncompatibleRepositories,
> -            repo.add_fallback_repository, base)
> -
>
> bb:approve

Thanks very much for the prompt review.

-- 
Martin <http://launchpad.net/~mbp/>