Code review comment for lp:~wence/bzr/check-clean

Thanks Lawrence! This change looks like the right approach.

+ if template:
+ include_history = True
             include_file_revisions = True
+ if '{clean}' in template:
+ check_clean = True

Looking at bzrlib.version_info_formats.format_custom the other two options are ignored in this case anyway, so you could collapse the check to:

    elif template and "{clean}" in template:
        check_clean = True

The version info command seems generally a little confused about which options are usefully combined, arguably `--template "{clean}"` shouldn't work at all unless `--check-clean` is also passed, but as your branch shows it's easy enough to do the right thing. The templating format is pretty simplistic so a basic contains substring check is correct.

There are three things needed before this can land.

1) Describe the fix and credit yourself in doc/en/release-notes/bzr-2.5.txt - see doc/developers/HACKING.txt for details but basically add an entry under the 2.5b3 Bug Fixes section, sorted alphabetically.

2) The hard bit, this really needs a test so we don't regress this with future changes to version-info. I'm failing to come up with an elegant way to do this, but a test method in bzrlib.blackbox.test_version_info which does run_bzr with a template not containing "{clean}" is the basic setup. The best idea I've had from there is doing before run_bzr something like:

    def should_not_be_called(self):
        raise AssertionError("Method on %r should not have been used" % (self,))
    self.overrideAttr(version_info_formats.VersionInfoBuilder,
        "_extract_file_revisions", should_not_be_called)
    out = self.run_bzr(["version-info", "--custom", "--template r{revno}\\n"])
    self.assertEqual("r0\n", out)

This is ugly monkey patching depending on a private method and is vulnerable to refactoring, but there don't seem to be any better placed hooks.

You'll want to check that the test fails without your fix, and passes with it, using `./bzr selftest -s bb.test_version_info` or similar.

3) Execute <http://www.canonical.com/contributors> copied to Martin Pool.