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
Attachment:
pgpbo2iHNhduj.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel