[rfc] [merge] removal of support for reverse changeset application

John A Meinel john at arbash-meinel.com
Thu Dec 22 22:57:43 GMT 2005


Denys Duchier wrote:
> John A Meinel <john at arbash-meinel.com> writes:
> 
>> I believe Aaron's original idea was deeper seated. He was thinking to
>> change the actually ChangesetEntry objects, so that their "apply()"
>> functions would change both the physical contents on disk, and the
>> inventory.
> 
> That's more or less what I intended to do.  I am just moving in this
> direction carefully - verifying with each modification that I am still
> computing the same information as before.
> 
>> The nice part about doing it that way, is it maintains the separation
>> between the lower and upper layers.
>> It means that ChangeExec.apply() can set/unset the 'executable="yes"'
>> flag, rather than having apply_changeset_to_inventory understand what
>> ChangeExec is doing, and how to apply it to the inventory.
> 
> As I said, the separate function was merely a device convenient to me
> during development.  It is only transitory.
> 
> While we are on the subject:  I find it odd that changeset entries
> have explicit object-based representations for contents changes and
> metadata changes, but not for tree changes.

I assume for this last one you mean adds/deletes/renames.

I think the idea is the Content object has a pre-path and a post-path.
If there is no pre, that means it is an add. If there is no post, then
it is a delete, etc.

I'm guessing Aaron's idea is that you have to have a path to define what
you are working on. (I think BaZing didn't track file-ids in the same
way, you'll probably notice a lot of the merge code is path based rather
than id based). By adding a second path, you now have the ability to
represent add/delete/rename, so it is not really necessary to add
another mechanism.
But I'm guessing, as I didn't do the implementing.

> 
> Cheers,
> 
> --Denys
> 

Some comments on your changes before I give them a second +1 and merge
it into my integration branch.

Why did you remove the Composition code? I'm guessing it wasn't being
used, but other than having specific discussions with Aaron, I wouldn't
want to see them removed. Now I believe you have talked to Aaron, and he
has approved your changes, but your short description doesn't include
some of these changes, so I just wanted a short discussion about it.

Also, I will agree with Aaron that some of the whitespace is not
correct. The official statement is that top-level code has 2 blank
lines, and nested code has 1.
So you have:

class A(object):
    """docstring"""

    def __init__(self):
        pass

    def foo(self, a):
        """docstring summary.

        more information here.
        :param a: Description of a
        :return: Description of return
        """
        aoeu
        aoeu
        aoeu #One space follows

    def baz(self, b):
        return b


class B(object): # Two spaces above
    pass


def do_something():
    pass


def do_something_else():
    pass


etc.

Other than some minor whitespace stuff, and wanting a brief description
as to why composition code was removed, I'm +1.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051222/aea3bbdc/attachment.pgp 


More information about the bazaar mailing list