[MERGE] More pack supporting transport fixes

[Martin Pool]

On 9/3/07, Robert Collins <robertc at robertcollins.net> wrote:
> 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.

This is a case where we have an interface that allows for a lot of
variation by implementers, but we also have specific expectations
about how the standard one will behave.  I'm suggesting that as well
as testing the standard interface, you should also test the specific
implementation behaves just as you expect.  Also, because most of the
code added is algorithmic not at all io-related, it would be nicer to
test it as a pure function.


-- 
Martin