Code review comment for lp:~vila/bzr/320119-exclude-ancestry

You have a few conflicts because this branch waited so long for a review. Sorry :(

You define a new stop_rule for iter_merge_sorted_revisions, but you haven't added it to the docstring. Please fix that.

It's a shame that make_branch_with_alternate_ancestries is duplicated in test_log and per_branch.test_iter_merge_sorted_revisions. Perhaps make it a function and have the latter import it from the former? Please at least add comments in both places noting the duplication.

(And further, it's a shame that the description of the ancestry is duplicated within that code... if only there were some way to do "make_branch_from_ascii_art_ancestry".)

Other than those, this seems ok to me. Once you fix those issues you can consider my vote upgraded to Approve.

I get the feeling that the next time we are tempted to another parameter to log we should think about refactoring the way we pass log options through the various layers (maybe with a LogOptions object? Or maybe Robert's idea about an algebra...), but this is ok for now.