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

Revision history for this message
Martin Packman (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.

« Back to merge proposal