[MERGE] 'bzr pack' tells the index builder to optimize

Andrew Bennetts andrew.bennetts at canonical.com
Wed Oct 22 06:44:20 BST 2008


bb:tweak

The tests feel slightly weak, because they are mainly about testing that
a flag is or isn't set on an object, rather than testing that
for_size=True and friends actually cause a change in behaviour.  I'm
don't think it's easy fix that, so I'm fine with this, but I felt I
ought to mention it.

Otherwise, just one comment:

John Arbash Meinel wrote:
[...]
> === modified file 'bzrlib/repofmt/pack_repo.py'
> --- bzrlib/repofmt/pack_repo.py	2008-10-01 05:40:45 +0000
> +++ bzrlib/repofmt/pack_repo.py	2008-10-16 18:58:22 +0000
> @@ -971,6 +971,21 @@
>          # TODO: combine requests in the same index that are in ascending order.
>          return total, requests
>  
> +    def open_pack(self):
> +        """Open a pack for the pack we are creating."""
> +        new_pack = NewPack(self._pack_collection._upload_transport,
> +            self._pack_collection._index_transport,
> +            self._pack_collection._pack_transport, upload_suffix=self.suffix,
> +            file_mode=self._pack_collection.repo.bzrdir._get_file_mode(),
> +            index_builder_class=self._pack_collection._index_builder_class,
> +            index_class=self._pack_collection._index_class)
> +
> +        new_pack.revision_index.set_optimize(for_size=True)
> +        new_pack.inventory_index.set_optimize(for_size=True)
> +        new_pack.text_index.set_optimize(for_size=True)
> +        new_pack.signature_index.set_optimize(for_size=True)
> +        return new_pack
> +

That's a lot of duplication.  Why not up-call to the parent class?  i.e.
implement this as:

    def open_pack(self):
        """Open a pack for the pack we are creating."""
        new_pack = super(OptimisingPacker, self).open_pack()
        new_pack.revision_index.set_optimize(for_size=True)
        new_pack.inventory_index.set_optimize(for_size=True)
        new_pack.text_index.set_optimize(for_size=True)
        new_pack.signature_index.set_optimize(for_size=True)
        return new_pack
        
-Andrew.




More information about the bazaar mailing list