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

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

On Fri, 2010-08-06 at 19:58 +0000, Martin [gz] 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.
>
> Just to note, until I rewrote it in r4792.7.3 from_lines did in fact
> require a list despite the docstring claiming it took an iterable, so
> changing the contract to actually require a list is unlikely to cause
> any kind of regression.
Ahh, thanks for letting me know. I thought that was the case so was
surprised to find it was using an iterator now.

Cheers,

Jelmer

« Back to merge proposal