[Spice-devel] [spice-gtk PATCH 1/3 v2] spicy-connect: Connect dialog changed to window
Christophe Fergeau
cfergeau at redhat.com
Thu Jun 11 08:41:01 PDT 2015
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>
More information about the Spice-devel
mailing list