[virt-tools-list] [PATCH virt-viewer v2] Update usage of GObject private structures

Christophe Fergeau cfergeau at redhat.com
Mon Feb 11 17:24:37 UTC 2019


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 <cfergeau at redhat.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190211/a065b7a9/attachment.sig>


More information about the virt-tools-list mailing list