On Tue, Dec 18, 2012 at 08:30:16PM +0200, Zeeshan Ali (Khattak) wrote:
> On Tue, Dec 18, 2012 at 8:15 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> > Actually, OsinfoInstallScript does not expose an
> > OsinfoInstallConfigParamList as this class is introduced by this series.
> > Not having this class in the first place is one of the reasons I assumed
> > this part of the code may need some improvements...
>
> I did indeed forget that fact that the list itself is not being
> exposed as is but rather InstallScript only provides methods to access
> params in the list.
The list is exposed, but as a GList, not as an
OsinfoInstallConfigParamList, which is what would be consistent with the
rest of the libosinfo API. Part of this series fixes that by adding
an OsinfoInstallConfigParamList class and making use of it.
If something makes some code redundant in this series, and could cause some
of the osinfo_install_script_*_config_param_* methods to become obsolete,
it's this change ("OsinfoInstallScript: Use an OsinfoInstallConfigParamList").
And this one really is a worthwhile cleanup.
>
> > We have various methods on the OsinfoInstallScript class to handle
> > OsinfoInstallConfigParam, but they look out of place to me, especially
> > after the introduction of the OsinfoInstallConfigParamList class. I
> > wouldn't mind deprecating these methods, they are probably not widely used
> > anyway.
>
> None of the libosinfo API is yet widely used. The main app that uses
> libosinfo is Boxes and we are using the methods in question in there.
Fwiw, I could find one single use of osinfo_script_get_config_param, so
whatever happens, it should not be too hard to adjust...
>
> As I said before, I don't think there is a compelling reason to
> deprecate APIs in this case. Hence the reason I wanted Daniel to make
> the decision here.
You are the one saying we should deprecate some methods, and at the same
time saying we should not be deprecating things.
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.
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.
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.
Christophe
Attachment:
pgp8N0L_r8I93.pgp
Description: PGP signature
_______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
