Re: [PATCH v2 3/5] NMSettingProxy added to libnm-core

[Atul Anand <atulhjp gmail com>]

On 7/27/16, Beniamino Galvani <bgalvani redhat com> wrote:
> On Tue, Jul 12, 2016 at 05:57:18PM +0530, Atul Anand wrote:
> > libnm-core has been expanded to include proxy settings which clients
> > like nm-connection-editor use to configure proxy in PacRunner. It
> > offers three modes i.e 'none', 'auto' and 'manual' and according take
> > data to configure PacRunner. The modes matches on the PacRunner side too.
>
> >  /**
> > + * nm_connection_get_setting_proxy:
> > + * @connection: the #NMConnection
> > + *
> > + * A shortcut to return any #NMSettingProxy the connection might
> > contain.
> > + *
> > + * Returns:an #NMSettingProxy if the connection contains one, otherwise
> > %NULL
>
> Can you please add the comment tag:
>
>   * Since: 1.4
>
> to every new function which is part of the libnm API?
>
>  NMSettingPppoe *           nm_connection_get_setting_pppoe
> (NMConnection *connection);
>  +NMSettingProxy *           nm_connection_get_setting_proxy
> (NMConnection *connection);
>
> And also add NM_AVAILABLE_IN_1_4 before declaration of such functions?
>
> > --- a/libnm-core/nm-core-types.h
> > +++ b/libnm-core/nm-core-types.h
> > @@ -43,6 +43,7 @@ typedef struct _NMSettingGsm              NMSettingGsm;
> >  typedef struct _NMSettingInfiniband       NMSettingInfiniband;
> >  typedef struct _NMSettingIPConfig         NMSettingIPConfig;
> >  typedef struct _NMSettingIPTunnel         NMSettingIPTunnel;
> > +typedef struct _NMSettingProxy            NMSettingProxy;
> >  typedef struct _NMSettingIP4Config        NMSettingIP4Config;
> >  typedef struct _NMSettingIP6Config        NMSettingIP6Config;
> >  typedef struct _NMSettingMacvlan          NMSettingMacvlan;
>
> Please keep this in alphabetical order.
>
> > +typedef enum {
> > +    NM_SETTING_PROXY_METHOD_NONE = 0,
> > +    NM_SETTING_PROXY_METHOD_AUTO,
> > +    NM_SETTING_PROXY_METHOD_MANUAL
> > +} NMSettingProxyMethod;
>
> Can you add a documentation comment to this public enum?
>
> Beniamino

Sure . Coming up with Fixes suggested by both of you .

Thanks