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

Re: [virt-tools-list] [libosinfo v2 5/8] InstallConfigParam:policy should not be writable



On Fri, Nov 09, 2012 at 04:14:21PM +0100, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> This actually not only breaks ABI but also the API: we remove one argument
> of _new() function. The prop getter is the main part of this API that an
> app will be using if its using this API at all so I think its worth it to
> correct this now  (while we are causing other breakage here: See next
> patch in this series).

This property is CONSTRUCT_ONLY anyway, I think this sort of construct is
a fairly typical of passing arguments when creating a gobject.

Christophe

> 
> I am also not certain that _new() should be part of the public API.
> ---
>  osinfo/osinfo_install_config_param.c | 18 ++----------------
>  osinfo/osinfo_install_config_param.h |  2 +-
>  osinfo/osinfo_loader.c               |  6 ++++--
>  3 files changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/osinfo/osinfo_install_config_param.c b/osinfo/osinfo_install_config_param.c
> index c6366c2..6d65c9b 100644
> --- a/osinfo/osinfo_install_config_param.c
> +++ b/osinfo/osinfo_install_config_param.c
> @@ -69,13 +69,6 @@ osinfo_install_config_param_set_property(GObject *object,
>                                      OSINFO_INSTALL_CONFIG_PARAM_PROP_NAME,
>                                      g_value_get_string(value));
>              break;
> -        case PROP_POLICY:
> -            {
> -            osinfo_entity_set_param(OSINFO_ENTITY(config_param),
> -                                    OSINFO_INSTALL_CONFIG_PARAM_PROP_POLICY,
> -                                    g_value_get_string(value));
> -            break;
> -            }
>          default:
>              /* We don't have any other property... */
>              G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
> @@ -166,7 +159,6 @@ osinfo_install_config_param_class_init (OsinfoInstallConfigParamClass *klass)
>                                  "Policy",
>                                  _("Parameter policy"),
>                                  NULL,
> -                                G_PARAM_WRITABLE |
>                                  G_PARAM_READABLE |
>                                  G_PARAM_CONSTRUCT_ONLY |
>                                  G_PARAM_STATIC_NAME |
> @@ -176,7 +168,6 @@ osinfo_install_config_param_class_init (OsinfoInstallConfigParamClass *klass)
>                                      PROP_POLICY,
>                                      pspec);
>  
> -
>      g_klass->finalize = osinfo_install_config_param_finalize;
>  
>      g_type_class_add_private (klass, sizeof (OsinfoInstallConfigParamPrivate));
> @@ -197,19 +188,14 @@ osinfo_install_config_param_init (OsinfoInstallConfigParam *config_param)
>  /**
>   * osinfo_install_config_param_new:
>   * @name: the configuration parameter name
> - * @policy: the configuration parameter policy
>   *
>   * Construct a new configuration parameter to a #OsinfoInstallScript.
>   *
>   * Returns: (transfer full): the new configuration parameter
>   */
> -OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name,
> -                                                          const gchar *policy)
> +OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name)
>  {
> -    return g_object_new(OSINFO_TYPE_INSTALL_CONFIG_PARAM,
> -                        "name", name,
> -                        "policy", policy,
> -                        NULL);
> +    return g_object_new(OSINFO_TYPE_INSTALL_CONFIG_PARAM, "name", name, NULL);
>  }
>  
>  /**
> diff --git a/osinfo/osinfo_install_config_param.h b/osinfo/osinfo_install_config_param.h
> index d588616..930614d 100644
> --- a/osinfo/osinfo_install_config_param.h
> +++ b/osinfo/osinfo_install_config_param.h
> @@ -69,7 +69,7 @@ struct _OsinfoInstallConfigParamClass
>  
>  GType osinfo_install_config_param_get_type(void);
>  
> -OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name, const gchar *policy);
> +OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name);
>  
>  const gchar *osinfo_install_config_param_get_name(const OsinfoInstallConfigParam *config_param);
>  
> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> index 4ad2d72..838b541 100644
> --- a/osinfo/osinfo_loader.c
> +++ b/osinfo/osinfo_loader.c
> @@ -582,8 +582,10 @@ static void osinfo_loader_install_config_param(OsinfoLoader *loader,
>      for (i = 0 ; i < nnodes ; i++) {
>          gchar *name = (gchar *)xmlGetProp(nodes[i], BAD_CAST "name");
>          gchar *policy = (gchar *)xmlGetProp(nodes[i], BAD_CAST "policy");
> -        OsinfoInstallConfigParam *param =
> -            osinfo_install_config_param_new(name, policy);
> +        OsinfoInstallConfigParam *param = osinfo_install_config_param_new(name);
> +        osinfo_entity_set_param(OSINFO_ENTITY(param),
> +                                OSINFO_INSTALL_CONFIG_PARAM_PROP_POLICY,
> +                                policy);
>          osinfo_install_script_add_config_param(OSINFO_INSTALL_SCRIPT(entity),
>                                                 param);
>  
> -- 
> 1.8.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list redhat com
> https://www.redhat.com/mailman/listinfo/virt-tools-list

Attachment: pgpcrH5iyPvZ3.pgp
Description: PGP signature


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