[MERGE] Remove change_entry from transform.py

Ian Clatworthy ian.clatworthy at internode.on.net
Fri Aug 24 02:39:08 BST 2007


Aaron Bentley wrote:
> John Arbash Meinel wrote:
>> John Arbash Meinel has voted approve.
>> Status is now: Approved
>> Comment:
>> A single release seems a bit short, but if you have a specific need for
>> it, that is ok.
> 
> I would really like to understand why he needs to actually remove it.
> It doesn't seem necessary to me.

That's a fair point. It may not be strictly necessary. I personally
don't care at all about change_entry(). What I care about is that it
uses (internal) APIs on InventoryEntry that I'd like to clean up:
_read_tree_state(), _unchanged() and _forget_tree_state().

I can always leave the existing code alone in inventory.py, implement
new methods and use those instead. The reason I had for not doing that
is it seems damn hard to change the responsibilities of repositories vs
inventories vs commit-builders as long as those 3 methods remain part of
the InventoryEntry interface. If the new APIs allow it, maybe
change_entry() can use those and the whole problem will go away. (My
expectation though is that InventoryEntrys will lose that responsibility
altogether delegating it to the CommitBuilder to return the interesting
data.)

I'm a big believer in being nice to client code and not churning things
too fast. Having said that:

* there remains 0 known users of the code two months after I Googled
  for it and asked about it on the list

* removing obsolete code is sometimes a better choice than letting
  the code base rot as the "API Breaks" section of NEWS shows. The 3
  methods I want to change are 30%-ish of the IE API.

Both Robert and Aaron have questioned my haste in wanting to remove this
code so I'll veto the patch. We can always dig it up again once the
desired layering is clearer and everyone feels it is the right decision.

Ian C.



More information about the bazaar mailing list