[Libosinfo] [PATCHv4 06/11] Add OsinfoInstallConfig:config-params property

[Christophe Fergeau]

On Wed, Dec 19, 2012 at 01:43:34AM +0200, Zeeshan Ali (Khattak) wrote:
> On Wed, Dec 19, 2012 at 12:33 AM, Christophe Fergeau
> <cfergeau at redhat.com> wrote:
> > You are the one saying we should deprecate some methods, and at the same
> > time saying we should not be deprecating things.
> 
> I've really had it with you generalizing my statements and taking them
> out of context.

Well, this was a bit tongue in cheek, but what I meant is I'm not pushing
to deprecate stuff. On the other hand, in this very mail, you are saying:
« I don't think we should be deprecating APIs without a good reason. »
and « Its like deprecating API silently. »

> 
> > As far as I'm concerned, after applying the whole patch series, the old API
> > is still working as always, I don't see reasons for it being broken any
> > time soon, and as far as I'm concerned the old API can stay.
> 
> As I said twice already, that will mean confusing the app developer
> with two competing APIs.

When I started working on these patches, I was very confused by
OsinfoInstallConfig VS OsinfoInstallConfigParam. They have similar names,
but they have no relationship whatsoever, you cannot get one from the
other, you are not using them in the same API calls, ... so the API is
confusing anyway.



> 
> > After looking at what Boxes does, it seems to make sense to have the
> > OsinfoInstallConfigParamList be available from both
> > OsinfoInstallScript and OsinfoInstallConfig depending on what is more
> > convenient for the user at a given time.
> 
> How would one be more convenient than the other?

If you have an OsinfoInstallScript object because you are setting up
'stuff' which you'll then use to create OsinfoInstallConfig(s), then it's
more convenient to get the OsinfoInstallConfigParamList from the script as
you haven't started creating your config. This is the way Boxes is using
it.
If you have already started your OsinfoInstallConfig and need to know which
parameters are mandatory, which are supported, ... then it's more
convenient to get the OsinfoInstallConfigParamList from the config as you
may not have as script handy.

> > I'd just like that we export osinfo_install_config_new_for_script and
> > promote its use in our docs as imo this will be useful moving forward, but
> > that's it.
> 
> Not deprecating existing API (now) while adding a competing API to it
> and promoting that in the docs is no solution. Its like deprecating
> API silently.


It's not a competing API, it's just convenience API that will ensure that
OsinfoInstallConfig:config-params is set on your newly created
OsinfoInstallConfig object. You can also set it by hand, or not set it at
all if you don't need it. What I'm suggesting complements and improves the
existing API, it does not really compete with it.


> Once again, I'll propose that Daniel makes the decision here. I don't
> know why you keep ignoring that suggestion.

I'm not ignoring it. I'm still refining my thoughts on this subject, and
slightly changing my point of view on the approach we should take in the
course of this email thread, so I'm echoing this here. Daniel's
input on that topic is obviously very welcome, but if we reach an agreement
without him, that's good as well.

To summarize my current take on this, both OsinfoInstallConfig:config-params
and OsinfoInstallScript:config-params are good to have. Depending on
the use case, both can be useful, so we should keep both.
We add two new methods osinfo_install_config_new_for_script() and
osinfo_install_config_get_config_params(). We don't deprecate anything.
osinfo_install_script_get_config_params() and osinfo_install_config_new()
will still be there as they are useful in different context.
And I don't see any huge confusion arising from that that cannot be fixed
by some basic documentation (I said 'basic', there is no complicated
confusing magic to understand there).


Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libosinfo/attachments/20121219/38739d83/attachment.sig>