Re: [gconfig v2 4/6] config: Add vnc listen getter/setter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi! Sorry for the delay, I had some exams.

_set_listen_list () is the only other name that comes into my mind.

I also think _set_listens () is a bit odd, but  _set_listen_nodes () doesn't sound that odd to me (maybe a bit too technical).

I think you should choose the name from those 3, because you have more experience, and I'll update and upload the remaining two patches right away.


All the best,
Alex

On Fri, Sep 2, 2016 at 5:17 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
On Wed, Aug 17, 2016 at 06:58:49PM +0300, Visarion Alexandru wrote:
> From: Visarion Alexandru <viorel.visarion@xxxxxxxxx>
>
> Learn to get/set the listen devices that vnc is using.
>
> When setting the listen devices, first remove the 'listen' attribute
> to avoid inconsistencies checks between the 'listen' attribute and the
> 'address' attribute of the 'listen' node.
> ---
>  .../libvirt-gconfig-domain-graphics-vnc.c          | 84 ++++++++++++++++++++++
>  .../libvirt-gconfig-domain-graphics-vnc.h          |  4 ++
>  libvirt-gconfig/libvirt-gconfig.sym                |  2 +
>  3 files changed, 90 insertions(+)
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
> index fc26bb9..46f3a4b 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c
> @@ -120,6 +120,90 @@ void gvir_config_domain_graphics_vnc_set_port(GVirConfigDomainGraphicsVnc *graph
>                                                 NULL);
>  }
>
> +/**
> + * gvir_config_domain_graphics_vnc_set_listen_devices:
> + * @graphics: a #GVirConfigDomainGraphicsVnc
> + * @listens: (in) (element-type LibvirtGConfig.DomainGraphicsListen):
> + *
> + * This method is used to set the various listen nodes a #GVirConfigDomainGraphicsVnc
> + * device can handle.
> +*/
> +void gvir_config_domain_graphics_vnc_set_listen_devices(GVirConfigDomainGraphicsVnc *graphics,
> +                                                        GList *listens)

Patch is good overall, but the naming needs improving, these <listen>
nodes are not devices. I'm not exactly sure how to name the API though.
_set_listen_nodes() is the only thing which comes to mind, and I don't
think it's great. Another options would be just _set_listens(), but
that's odd too. Any better suggestion?

Apart from this naming issue (which has to be fixed before pushing),

Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

Christophe



--
Visarion-Mingopol Alexandru-Viorel
Telefon : 0729614060
Best Bucuresti
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]