Code review comment for lp:~jelmer/bzr/merge-dir-prober

Hi Aaron,

On Thu, 2010-08-05 at 02:42 +0000, Aaron Bentley wrote:
> This seems fine in general, but I believe you're changing the signature
> of from_lines somewhat. Previously, it accepted an iterable, which
> could be a collection (i.e. list) or an iterator. Now, it seems a
> collection is mandatory because the iteration is repeated for every
> prober, but an iterator would be exhausted by the first prober.
>
> Mandating lists may increase memory use, though of course, a given line
> could be twelve gigabytes if we were really unlucky. At the very
> least, you should change the docstring if you change the signature.
>
> Because each prober is handled in sequence, you'll also be consuming
> the whole list of lines for every probe type, even if there's a header
> at the top of the file, indicating its kind.
>
> Perhaps it would be better to us a coroutine approach instead. For example:
That's a good point, for some reason I thought that it scaled with the
size of the file anyway. Using coroutines does indeed seem like a good
approach here.

Thanks for the review.

Cheers,

Jelmer