[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