[MERGE] Nuke versioned file add/get delta support, allowing easy simplification of unannotated Content, reducing memory copies and friction during commit on unannotated texts.

Robert Collins robertc at robertcollins.net
Thu Sep 6 02:39:41 BST 2007


On Wed, 2007-09-05 at 21:18 -0400, Martin Pool wrote:
> Martin Pool has voted tweak.
> Status is now: Conditionally approved
> Comment:
> 
> +class ExistingContent(BzrError):
> +
> +    """Added in bzrlib 0.92, used by VersionedFile.add_lines."""
> 
> The first line of the docstring ought to tell what this *is*, and that
> doesn't.  Can you please add something before saying what this exception
> means?

Oh yeah, oops.


> 
> +class PlainKnitContent(KnitContent):
> +    """Unannotated content."""
> +
> 
> This has the special behaviour of, when annotated, claiming that 
> everything
> came from a single version.  That seems like quite a nice facility
> but you should really say that's what happens.

Its actually quite bogus, I have preserved that behaviour from the
previous code - its at least more clear now that its doing it. I will
add a note to that effect to the class docstring.

> +        # Technically this is a case of LBYL, but:
> 
> Please expand the acronym; you can capitalize the phrase if you want.

ok.

> -class KnitContentTests(TestCase):
> +class KnitContentTestsMixin(object):
> 
> I would kind of prefer that you change these to scenarios instead, but I
> don't insist on it.

It really doesn't fit well where the scenario is less than a file in
size. I've seen you mention this in a few reviews, but I don't think the
infrastructure is where it needs to be to be doing it for lightweight
tiny interfaces like this.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070906/3970d19b/attachment.pgp 


More information about the bazaar mailing list