[Libosinfo] [PATCHv4 07/11] Automatically remap OsinfoInstallConfig values if needed

[Christophe Fergeau]

On Tue, Dec 18, 2012 at 03:31:51AM +0200, Zeeshan Ali (Khattak) wrote:
> On Mon, Dec 17, 2012 at 11:07 PM, Christophe Fergeau
> <cfergeau at redhat.com> wrote:
> > Now that OsinfoInstallConfig has access to the
> > OsinfoInstallConfigParamList for the OsinfoInstallScript that is
> > being configured,
> 
> In the previous patch (06/11) you just added the setter/getter for
> config params to OsinfoInstallConfig but nothing yet sets the params.
> Either I'm totally confused or the ordering of these patches is
> somehow wrong.

Let's blame the commit log here, what the series is doing is
1) add the OsinfoInstallConfig:config-parameters property
2) use it in OsinfoInstallConfig _if it is set_
3) use OS specific values in OsinfoInstallScript
4) set OsinfoInstallConfig:config-parameters where needed

This commit is doing 2) even though the commit log can be misleading.
I can see 3) being misplaced, it could go last, but apart from this I think
the ordering is not that bad.

> > we can use the OsinfoDatamap that is optionally
> > set on a given parameter to automatically translate a value for
> > this parameter from a generic libosinfo value to an OS-specific one.
> 
> Assuming config params in OsinfoInstallScript has (or can has) access
> to datamaps, I wonder if there is any need to involve
> OsinfoInstallConfig at all here. The changes will be less intrusive
> that way AFAICT.

I've seen your followup to this email saying to disregard this comment. For
what it's worth, I've already detailed in
https://www.redhat.com/archives/virt-tools-list/2012-December/msg00288.html
why I think adding config params to OsinfoInstallConfig is a good move
regardless of the datamaps stuff.

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/20121218/cb2669e8/attachment.sig>