Merge lp:~lifeless/testresources/bug-271619 into lp:~testresources-developers/testresources/trunk
Proposed by
Robert Collins
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~lifeless/testresources/bug-271619 |
Merge into: | lp:~testresources-developers/testresources/trunk |
Diff against target: | None lines |
To merge this branch: | bzr merge lp:~lifeless/testresources/bug-271619 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jonathan Lange | Approve | ||
James Henstridge | Pending | ||
testresources developers | Pending | ||
Review via email: mp+7610@code.launchpad.net |
To post a comment you must log in.
I think this patch is fundamentally correct and adds a useful feature.
However, I think the docstring for `reset` should be changed so that the first line says something like, "Return a clean version of 'old_resource'". That is, it should describe what the method does.
The docstring should also provide some hints as to *why* one might want to override the method. One way to do this would be to follow-up the "By default" sentence with "If there is a faster way of resetting the resource than cleaning it and remaking it, you probably want to override this method to do that." Or something along those lines.
Thanks for the patch & for dealing with this long-dormant bug.
jml