Merge lp:~wence/bzr/check-clean into lp:bzr
Proposed by
Lawrence Mitchell
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Martin Packman | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 6245 | ||||
Proposed branch: | lp:~wence/bzr/check-clean | ||||
Merge into: | lp:bzr | ||||
Diff against target: |
66 lines (+22/-3) 3 files modified
bzrlib/cmd_version_info.py (+6/-1) bzrlib/tests/blackbox/test_version_info.py (+12/-2) doc/en/release-notes/bzr-2.5.txt (+4/-0) |
||||
To merge this branch: | bzr merge lp:~wence/bzr/check-clean | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Approve | ||
Martin Packman (community) | Approve | ||
Review via email: mp+80657@code.launchpad.net |
Commit message
Only imply --check-clean for version-info if {clean} is in the template
To post a comment you must log in.
Thanks Lawrence! This change looks like the right approach.
+ if template:
include_ file_revisions = True
+ include_history = 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): "Method on %r should not have been used" % (self,)) overrideAttr( version_ info_formats. VersionInfoBuil der,
"_extract_ file_revisions" , should_ not_be_ called) bzr(["version- info", "--custom", "--template r{revno}\\n"]) assertEqual( "r0\n", out)
raise AssertionError(
self.
out = self.run_
self.
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/contributor s> copied to Martin Pool.