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

[Zeeshan Ali (Khattak)]

On Wed, Dec 19, 2012 at 11:16 AM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> 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. »

Yes, I *was* saying that what you are proposing really is nothing
short of deprecating existing API. Stress on the word *was* cause I
realized now that you have slightly changed your opinion here (see
below).

>> > 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.

I understand and agree that this API is confusing but by keeping it
and adding the same API on another class will cause a lot more
confusion. That would be actually contrary to what you are trying to
achieve here.

>> > 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, ...

That is dependent on the script and config alone can not tell you
that. The only way config can tell you this is to get this information
from a script.

> then it's more
> convenient to get the OsinfoInstallConfigParamList from the config as you
> may not have as script handy.

You can't have this information without a script instance at hand for
the reason I mentioned above. So I'm sorry but I don't see any
convenience being added.

>> > 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.

Why would we want an app to be doing that?

> What I'm suggesting complements and improves the
> existing API, it does not really compete with it.

So I take it that you have changed your opinion slightly on this cause
you were the first to call these two approaches "competing" in this
thread?

>> 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.

Good, sorry I misunderstood then.


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124