[MERGE] Refactor commit to prepare for population by tree walking

Ian Clatworthy ian.clatworthy at internode.on.net
Wed Jul 18 07:07:07 BST 2007


Take 3 on this bit of commit refactoring ...

Updated bundle attached together with a diff from the previous one. I've
corrected the two naming issues identified below and incorporated your
feedback on the 3rd issue (existing code is overkill) as a comment. That
feels/sounds a bit lame but I'm not sure how best to change that code to
reflect your point and it's covering a low frequency corner case that I
feel is lower priority than other changes commit needs.

Is this acceptable now?

Aaron Bentley wrote:

> This refactoring looks odd.  For one thing, the root hasn't really been
> added.  For another, root_added_already is redundant with
> self.builder.record_root_entry.

OK. Naming and code cleaned up.

>> +        # If specific files/directories were nominated, it is possible
>> +        # that some data from outside those needs to be preserved from
>> +        # the basis tree. For example, if a file x is moved from out of
>> +        # directory foo into directory bar and the user requests
>> +        # ``commit foo``, then information about bar/x must also be
>> +        # recorded.
> 
> ^^^ This implies that bar knows what its children are, which isn't
> really accurate.  Strictly speaking, bar only needs to be committed if
> it wasn't a directory in the basis tree-- the current implementation is
> overkill.

Comment added to that effect. I'm not overly keen to change this right
now as its low frequency, in production (not new code) and I'm not sure
how best to do it. Moving to InventoryDeltas might make it obsolete anyhow?

>> +            # Record an entry for this item
>> +            # Note: I don't particularly want to have the existing_ie
> 
> It's confusing that you're referring to new_ie as existing_ie here.

A hang over from the current code. I'm not exactly sure why new_ie was
chosen as the variable name but it's now existing_ie throughout that
routine.

Ian C.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: commit-prepare-populate-alternatives-3.patch
Type: text/x-patch
Size: 93868 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070718/dde07d45/attachment-0002.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: commit-prepare.diff
Type: text/x-patch
Size: 3531 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070718/dde07d45/attachment-0003.bin 


More information about the bazaar mailing list