Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
On 18/01/17 11:06, Christophe Fergeau wrote:
> On Wed, Jan 18, 2017 at 10:56:59AM -0200, Eduardo Lima (Etrunko) wrote:
>> On 18/01/17 10:53, Christophe Fergeau wrote:
>>> On Wed, Jan 18, 2017 at 10:08:42AM -0200, Eduardo Lima (Etrunko) wrote:
On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
> 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)
>>> ---
>>> 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.
Thinking better about this, it is not possible to make it a construct
only, because the ovirt object is created later on the process, when
remote-viewer has already started. Also, there is the possibility of the
pointer being set or not, depending on how the application was invoked.
>>>
>>> I don't understand what you mean here. In the part of the code quoted
>>> above, a RemoteViewerISOListDialog instance is created, and right after
>>> that, we set self->priv->foreign_menu on that instance, so in this case,
>>> a CONSTRUCT_ONLY property would be fine.
>>
>> The ovirt-foreign-menu property is installed as a member of RemoteViewer
>> object, not RemoteViewerISODialog.
>
> Oh, what I was saying is that
>
> +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_D
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
On Wed, Jan 18, 2017 at 10:56:59AM -0200, Eduardo Lima (Etrunko) wrote:
> On 18/01/17 10:53, Christophe Fergeau wrote:
> > On Wed, Jan 18, 2017 at 10:08:42AM -0200, Eduardo Lima (Etrunko) wrote:
> >> On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
> >>> 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)
> > ---
> > 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.
> >>
> >>
> >> Thinking better about this, it is not possible to make it a construct
> >> only, because the ovirt object is created later on the process, when
> >> remote-viewer has already started. Also, there is the possibility of the
> >> pointer being set or not, depending on how the application was invoked.
> >
> > I don't understand what you mean here. In the part of the code quoted
> > above, a RemoteViewerISOListDialog instance is created, and right after
> > that, we set self->priv->foreign_menu on that instance, so in this case,
> > a CONSTRUCT_ONLY property would be fine.
>
> The ovirt-foreign-menu property is installed as a member of RemoteViewer
> object, not RemoteViewerISODialog.
Oh, what I was saying is that
+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_objec
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
On 18/01/17 10:53, Christophe Fergeau wrote:
> On Wed, Jan 18, 2017 at 10:08:42AM -0200, Eduardo Lima (Etrunko) wrote:
>> On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
>>> 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)
> ---
> 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.
>>
>>
>> Thinking better about this, it is not possible to make it a construct
>> only, because the ovirt object is created later on the process, when
>> remote-viewer has already started. Also, there is the possibility of the
>> pointer being set or not, depending on how the application was invoked.
>
> I don't understand what you mean here. In the part of the code quoted
> above, a RemoteViewerISOListDialog instance is created, and right after
> that, we set self->priv->foreign_menu on that instance, so in this case,
> a CONSTRUCT_ONLY property would be fine.
The ovirt-foreign-menu property is installed as a member of RemoteViewer
object, not RemoteViewerISODialog.
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
On Wed, Jan 18, 2017 at 10:08:42AM -0200, Eduardo Lima (Etrunko) wrote:
> On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
> > 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)
> >>> ---
> >>> 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.
>
>
> Thinking better about this, it is not possible to make it a construct
> only, because the ovirt object is created later on the process, when
> remote-viewer has already started. Also, there is the possibility of the
> pointer being set or not, depending on how the application was invoked.
I don't understand what you mean here. In the part of the code quoted
above, a RemoteViewerISOListDialog instance is created, and right after
that, we set self->priv->foreign_menu on that instance, so in this case,
a CONSTRUCT_ONLY property would be fine.
Christophe
signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
> 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)
>>> ---
>>> 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.
Thinking better about this, it is not possible to make it a construct
only, because the ovirt object is created later on the process, when
remote-viewer has already started. Also, there is the possibility of the
pointer being set or not, depending on how the application was invoked.
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
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)
>> ---
>> 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
etru...@redhat.com
signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property
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)
> ---
> 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).
> +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.
Christophe
signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
