On 17/01/17 14:00, Christophe Fergeau wrote:
> On Fri, Jan 13, 2017 at 07:11:07PM -0200, Eduardo Lima (Etrunko) wrote:
>> The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
>> we make it acessible via property to avoid interdependency between
>> objects.
>>
>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
>> ---
>> src/remote-viewer-iso-list-dialog.c | 31 +++++++++++++++++++++++--------
>> src/remote-viewer-iso-list-dialog.h | 2 +-
>> src/remote-viewer.c | 37 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c
>> index 0fbea28..858719c 100644
>> --- a/src/remote-viewer-iso-list-dialog.c
>> +++ b/src/remote-viewer-iso-list-dialog.c
>> @@ -24,6 +24,7 @@
>>
>> #include "remote-viewer-iso-list-dialog.h"
>> #include "virt-viewer-util.h"
>> +#include "ovirt-foreign-menu.h"
>>
>> G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG)
>>
>> @@ -33,11 +34,16 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE
>> struct _RemoteViewerISOListDialogPrivate
>> {
>> GtkWidget *stack;
>> + OvirtForeignMenu *foreign_menu;
>> };
>>
>> static void
>> remote_viewer_iso_list_dialog_dispose(GObject *object)
>> {
>> + RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
>> + RemoteViewerISOListDialogPrivate *priv = self->priv;
>> +
>> + g_clear_object(&priv->foreign_menu);
>> G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
>> }
>>
>> @@ -94,13 +100,22 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
>> }
>>
>> GtkWidget *
>> -remote_viewer_iso_list_dialog_new(GtkWindow *parent)
>> +remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
>> {
>> - return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
>> - "title", _("Change CD"),
>> - "transient-for", parent,
>> - "border-width", 18,
>> - "default-width", 400,
>> - "default-height", 300,
>> - NULL);
>> + GtkWidget *dialog;
>> + RemoteViewerISOListDialog *self;
>> +
>> + g_return_val_if_fail(foreign_menu != NULL, NULL);
>> +
>> + dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
>> + "title", _("Change CD"),
>> + "transient-for", parent,
>> + "border-width", 18,
>> + "default-width", 400,
>> + "default-height", 300,
>> + NULL);
>> +
>> + self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
>> + self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
>
> Fwiw, a construct-only GObject property would be more expected (but you
> can keep it this way i f you think it's better).
No objections, I will change it.
>
>> + return dialog;
>> }
>> diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h
>> index def841b..8b936f5 100644
>> --- a/src/remote-viewer-iso-list-dialog.h
>> +++ b/src/remote-viewer-iso-list-dialog.h
>> @@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
>>
>> GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
>>
>> -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
>> +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu);
>>
>> G_END_DECLS
>>
>
> The remote-viewer-iso-list-dialog.[ch] changes are odd in this patch,
> they don't have much to do with the added
> RemoteViewer::ovirt-foreign-menu property at this point.
> I think this part of the patch can probably be merged with the previous
> one, the property addition can be done before that.
>
Thanks for the review.
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
