Code review comment for lp:~spiv/bzr/inventory-delta

Revision history for this message
John A Meinel (jameinel) wrote :

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

...

>>>> + if not unstacked_repo._format.supports_chks:
>>>> + # these assertions aren't valid for groupcompress repos, which may
>>>> + # transfer data than strictly necessary to avoid breaking up an
>>>> + # already-compressed block of data.
>>>> + self.assertFalse(unstacked_repo.has_revision('left'))
>>>> + self.assertFalse(unstacked_repo.has_revision('right'))
>>> ^ please check the comment, its not quite clear.
>> s/transfer data/transfer more data/
>>
>> I'm not 100% certain that's actually true? What do you think with the adjusted
>> comment?
>
> Big alarm bell. gc repos aren't permitted to do that for the revision
> objects, because of our invariants about revisions. Something else is
> wrong - that if _must not_ be needed.

Actually gc repos may send more data in a group, but they don't
*reference* the data. So the comment is actually incorrect. They don't
reference any more data than the strictly necessary set, and something
like "has_revision" should *definitely* be failing.

John
=:->

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

iEYEARECAAYFAkp8NvQACgkQJdeBCYSNAAP4YgCcCmrttXg3iIomKS+JEsm5S6ih
pUMAoKpxevabz88K2yG5ZTx1zhB0s2Bc
=Yx5Q
-----END PGP SIGNATURE-----

« Back to merge proposal