Code review comment for lp:~salgado/bzr/bug-308122

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> 2009/12/23 John A Meinel <email address hidden>:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Guilherme Salgado wrote:
>>> So, to summarize, this is how it's going to look like, in the end:
>>>
>>> 1. cmd_unshelve will grow a 'preview' action
>>>
>>> 2. Unshelver.run() will accept an optional file descriptor, where a diff
>>> will be written when the preview action is specified. When the fd is not
>>> given it will fall back to ui_factory.make_output_stream().
>> I'm not sure whether it should be on run() or as a constructor argument.
>>
>> unshelver = Unshelver.from_args(..., to_file=self.outf)
>> or
>> Unshelver.run(to_file=self.outf)
>
> I haven't looked at the implementation yet but maybe it would be
> better to actually have Unshelver.preview(to_file=...)
>

so the Unshelver decides what is going to do in the
"Unshelver.from_args" which takes an 'action'. (dry-run, delete, and now
preview.)

I believe this is because Unshelver is meant to be a UI class, and not a
lower level implementation class. As in, you probably can't really
re-use Unshelver in gui code, as it is an implementation of a text ui
itself.

Which feels like we are missing the non-ui code (how *should* qunshelve
interact with the shelf if not via Unshelver.)

One hint to that is that the class is in "bzrlib.shelf_ui" and not
"bzrlib.shelf".

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksxhmsACgkQJdeBCYSNAAOq1wCglzDzRv5c1W1BaZy84PZ0dY3m
18UAoJJggi4ldHawXVYcdNCQer9xuqcv
=WZNo
-----END PGP SIGNATURE-----