On Tue, Dec 18, 2012 at 7:38 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> 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@xxxxxxxxxx> 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 _______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
