Hey, Looks good to me, 2 comments below:
On Thu, Feb 07, 2019 at 10:54:58AM -0200, Eduardo Lima (Etrunko) wrote: > diff --git a/src/remote-viewer-iso-list-dialog.c > b/src/remote-viewer-iso-list-dialog.c > index 3505211..8e25b72 100644 > --- a/src/remote-viewer-iso-list-dialog.c > +++ b/src/remote-viewer-iso-list-dialog.c > @@ -114,7 +109,7 @@ > remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass > *klass) > static void > remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) > { > - self->priv = DIALOG_PRIVATE(self); > + self->priv = remote_viewer_iso_list_dialog_get_instance_private(self); Is this really needed here when it's already done in remote_viewer_iso_list_dialog_init? (such a change would belong in a separate commit though) > gtk_stack_set_visible_child_full(GTK_STACK(self->priv->stack), > "iso-list", > GTK_STACK_TRANSITION_TYPE_NONE); > gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, > TRUE); > @@ -262,7 +257,8 @@ static void > remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) > { > GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self)); > - RemoteViewerISOListDialogPrivate *priv = self->priv = > DIALOG_PRIVATE(self); > + RemoteViewerISOListDialogPrivate *priv = self->priv = > + remote_viewer_iso_list_dialog_get_instance_private(self); Hiding the initialization of 'self->priv' in the middle of local variable declaration/initialization is not the most readable thing ever, this could be split too ;) Apart from this, looks good to me, Reviewed-by: Christophe Fergeau <cferg...@redhat.com>
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list