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

[Zeeshan Ali (Khattak)]

On Tue, Dec 18, 2012 at 7:38 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Tue, Dec 18, 2012 at 07:18:04PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Tue, Dec 18, 2012 at 6:43 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>> > In my opinion, the second approach is more convenient both for the library
>> > user and for us from a maintainance point of view, which explains my
>> > insistance on moving this way ;)
>>
>> Thanks for the summary. Thing is that we have already moved a lot
>> towards the first approach
>
> Are you saying the OsinfoInstallScript/OsinfoInstallConfig/
> OsinfoInstallConfigParam relationship was intentionally designed this way?
> Ie OsinfoInstallConfigParam and OsinfoInstallConfig are separate on
> purpose?

Yes.

>Until now my feeling was that this ended up being implemented
> this way, but that this was not a conscious decision, especially as this
> code didn't land very long ago.

AFAIK, the code in question has been in a separate branch/list for a
while and was thoroughly reviewed.

>> and to be able to move towards the second
>> approach, we must deprecate the API that are designed for the first
>> (existing) approach. Otherwise we'll just be confusing app developers
>> with this contradiction.
>
> I'm not exactly sure what API you have in mind exactly. All I'm suggesting
> is gradual improvement and having a clear idea of what belongs where. I
> don't think we have things to deprecate to achieve that.

I'm talking about having this config-params on both InstallScript and
InstallConfig. As you pointed out already, currently app has to look
at the params on the InstallScript to see which parameters the script
will be able to use and which ones it must be provided.

Exposing the same list on InstallConfig w/o deprecating it on the
InstallScript, is the change I'm talking about. I hope I was more
clear this time.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124