[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