[MERGE] Make log revision filtering pluggable.

[John Arbash Meinel]

John Arbash Meinel has voted comment.
Status is now: Semi-approved
Comment:
I'm a bit concerned about your api. You seem to have an iterator of 
iterators, which is a bit confusing in its nesting.

Like this:
+        def _convert():
+            for view in view_revisions:
+                yield (view, None, None)
+        log_rev_iterator = iter([_convert()])

If I understand it correctly you need to do a double for loop on that. 
Such as

for iterator in log_rev_iterator:
   for revision_id, revno, depth in iterator:
     do stuff

At a minimum, it would be nice to have it more clearly documented.

It also seems like the function would be:
make_log_rev_iterators (note the plural iterators).

And then "for rev_iter in revision_iterators:"
etc.

So... I really like it being a chain of filters. But I wonder if we 
would be a lot better served by having a chain of Classes rather than 
plain functions. At least at the moment, I have difficulty following 
what is really going on. I can sort of tease it out, but it doesn't 
really pass my "simplicity" filter.

Oh, and if "log_adapters" is going to be non-stable, I think it should 
be "_log_adapters" for now.


For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C1219353364.31549.149.camel%40lifeless-64%3E
Project: Bazaar