[Merge] lp:~salgado/bzr/bug-308122 into lp:bzr

Aaron Bentley aaron at aaronbentley.com
Thu Dec 24 06:39:44 GMT 2009

Hash: SHA1

John Arbash Meinel wrote:
> So to start with, there is a bit of confusion, because there is both
> bzrlib.shelf.Unshelver
> and
> bzrlib.shelf_ui.Unshelver

Absolutely.  That was plain creativity failure on my part.  I couldn't
think of another name, but UnshelverUI is better than exactly the same

> Anyway, there are actions like "show_changes" that seem relevant for
> both a gui and a text ui, that are only present on shelf_ui.Unshelver.
> Looking at it, there isn't a lot of code here, so it wouldn't be a lot
> of code duplication.

And I guess that's part of the problem-- there isn't a lot of code, so I
didn't think it was worth putting it into its own function or method.
But I guess it's useful to make the common case blindingly simple.

So for this case, we could implement
shelf.Unshelver.show_changes(change_reporter), or
shelf.ShelfManager.show_changes(change_reporter, shelf_id) or
WorkingTree.show_shelf_changes(change_reporter, shelf_id).

It's a bit sad to contemplate these, because they mean that we throw
away the merger and tree transform-- no chance to reuse them later for
actually applying the change, for example.

> I think the bigger issue is that you were trying to do something new
> with the shelf_ui code. Which is probably a good thing, but because it
> is different from the rest of the codebase, it isn't obvious how the
> pieces interact.

Well, here what I was doing was trying make all the DWIM stuff of the
commmand optional.  So you can directly construct an UnshelverUI,
specifying a tree, manager, and shelf_id, and then call "run" on it.

I don't see a lot on UnshelverUI that makes it unsuitable for use in
"qunshelve".  You'd want to be able to specify a custom ChangeReporter,
I suppose.  What else?

Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org


More information about the bazaar mailing list