Code review comment for lp:~ian-clatworthy/bzr/eol-update-bug

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

This patch fixes content filtering so that it works after most operations, not just after the initial branch/checkout. There were 2 code paths in transform.py that needed enhancing to check for content filters: one used by 'merging' and one used by 'reverting'. The merging code path is used by lots of operations including merge, pull, update and switch.

I've added several 'blackbox-style' tests as part of this patch. It probably wouldn't hurt to add some more for other operations, e.g. shelve/unshelve, though I'm optimistic that the 2 code paths above will cover 99% of them.

In any case, I'm submitting this for review now because:

1. Without these code changes, content filtering is effectively broken for most users.

2. The 2.0 code freeze is really close and I want to maximise the time available for review of what's fixed/tested already.

3. Those tests could come in a latter patch (after there's agreement on whether blackbox-style tests are acceptable here or not).

BTW, the bug report also mentions that after committing a tree with keywords expanded, the keywords aren't re-expanded after commit. I'm going to work on that issue now, either by adding a post-commit hook to the keywords plugin or by changing commit. The latter approach implies always paying a performance cost whether it's required or not so I'll try the post-commit approach first. Either way, I feel it's best to keep that issue out of scope of this patch.

« Back to merge proposal