Code review comment for lp:~abentley/launchpad/incremental-diffs

OK, I've done a review of the difftastic codebase and I see no reason why it can't be used in Launchpad. I do have one possible performance improvement, and a number of changes to make the tests more clear.

1. "foo" and "bar" are fine for filenames within the test branches,
but they really hurt comprehension when used as variable names
for the branches themselves. In the helper methods, can you call
the branches 'initial' and 'merged_in' or something that reflects
their history?

Outside the helper methods, you can call them whatever's
helpful. For instance, in test_mainline_diff, I believe
'foo' (aka 'initial') could be called 'mainline' and 'bar' (aka
'merged_in') could be called 'ignored'.

2. typo: "emtpy commit"

3. test_diff_ignore_branches calls
create_unignored_merge_scenario() to do its setup. This sounds
like a contradiction, but we're actually checking that only
*specified* branches are ignored. A docstring would make it clear
that we're checking that the diff ignores the merge of the "bar"
branch, but not the merge of the "unignored" branch. Similarly
for ignore_old_revisions--what's being ignored and what's being
included? It's hard to figure out.

Making the commits that are being ignored really obviously
destructive would also help.

3. In generate_diff.py#apply_merges(), do you need to call
merger.make_preview_transform and transform.get_preview_tree
within the for loop? Unless those methods modify 'merger'
in-place, I think they can be moved outside and run only once.