[MERGE] Improving check in myriad ways
John Arbash Meinel
john at arbash-meinel.com
Tue Feb 5 15:38:36 GMT 2008
Daniel Mark Watkins wrote:
> On Tue, 05 Feb 2008 08:33:36 -0500
> Aaron Bentley <aaron at aaronbentley.com> wrote:
>> Actually, there's no bundle.
> I suck. Now with 100% more bundle.
>
> Dan
>
Just a quick review comment. We can't accept this as-is because you are
changing the signature of "bzrlib.check.check". It used to take a branch
object, and now it takes a plain path string.
What you should do instead is create a new function "check_path"
perhaps. You can deprecate "check" itself, since "check_branch" is a
nicer name for what it is doing. But it should continue to work like before.
So something like:
@symbol_versioning.deprecated_function(symbol_versioning.one_three)
def check(branch, verbose):
# thunk into check_branch, which is what this function should have
# been called
return check_branch(branch, verbose)
def check_branch(branch, verbose):
... # The old code from 'check'
def check_path(path, verbose):
# Your new code for check.
Also, I think your ordering is a bit off. For example, I would do:
try:
tree = WorkingTree.open_containing(path)[0]
except ...
tree = None
try:
branch = ...
except ...:
branch = None
try:
repo = Repository.open()
except ...:
# Raise an error indicating there is nothing to check
else:
repo = branch.repository
except:
branch = tree.branch
repo = branch.repository
You wrote: +def _scan_for_branches(path):, but it only scans 1 level
deep, and it uses "open_containing" which is a bit odd considering you
wouldn't want to open a branch in the parent directory over and over again.
Either way, we now have Repository.find_branches() which you can use
rather than implementing it yourself.
John
=:->
More information about the bazaar
mailing list