[MERGE] More pack supporting transport fixes

Robert Collins robertc at robertcollins.net
Mon Sep 3 06:39:36 BST 2007


On Mon, 2007-09-03 at 01:16 -0400, Martin Pool wrote:
> Martin Pool has voted resubmit.
> Status is now: Resubmit
> Comment:
> It looks like the code that does the expansion is a pure function based 
> on only the requested offsets and the estimated page size.  I think it 
> would be cleaner to split that out to a method on Transport that only 
> does the expansion, and call that from readv.  You can then test 
> directly that the new method has exactly the behavior you expect for 
> particular page sizes and read requests.

so you mean that readv callers another helper - the problem with that is
that its legitimate for a given transport to decide to expand it in a
different manner. If on the other hand we say that transports are
permitted to do this, I would have done the expansion outside the
transport API in the index API instead, because that would have been
conceptually simpler - it was your suggestings that transport should
have the _opportunity_ to decide that led me to put it in readv in the
first place.

> I would say you now have an api break that plugins should now override 
> _readv not readv, unless they really know what they're doing.

This is true, so I'll add a note to NEWS in the API breaks section.

Anyhow, this patch is currently broken pending fixing the remote
transport support for this, so it will rot for a short while I suspect.

-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/20070903/409a52d8/attachment.pgp 


More information about the bazaar mailing list