[Spice-devel] [spice-gtk PATCH 1/3 v2] spicy-connect: Connect dialog changed to window

[Christophe Fergeau]

Hey,

On Tue, Jun 09, 2015 at 04:59:32PM +0200, Lukas Venhoda wrote:
> Connect dialog in spicy had no transinient parent.
> It didn't make much sense for it to be a dialog.
> 
> Changed the connect dialog to a window.
> Moved the dialog code to its own module.

I'd make the description a bit more complex, eg "Since the connect dialog in
spicy does not have a transient parent, it does not make much sense for
it to be a dialog. This commit changes the dialog to be a GtkWindow, and
moves the dialog code to its own file."

> 
> Changed response ID from ambiguous -1 and 0 to GTK_RESPONSE_ACCEPT
> and GTK_RESPONSE_REJECT.
> 
> Improved UX:
>    - ESC to close, ENTER to connect.
>    - Address and port are now required to connect.
>    - Focusing in entries will now unselect recent connection.
>       - If you rewrite something in entries, you can now reselect
>         connection in the recent chooser.

Having each of these changes in a separate commit would make the overall
series easier to review as the dialog -> window change is probably
fairly mechanical, the move to a new file should change very little
code, and then each of these improvements would be a very small commit,
which would be much easier to review for potential bug additions.
Commits 2 and 3 would belong to the commit moving the code to a new
file, as they logically belong together.

With everything in a single commit, the reviewer has to figure out to
what of these logical change each hunk belongs to, which is much harder
and time consuming :(

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150611/7ce0a636/attachment.sig>