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

[Zeeshan Ali (Khattak)]

On Tue, Dec 18, 2012 at 1:01 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Tue, Dec 18, 2012 at 11:03:24AM +0100, Christophe Fergeau wrote:
>> 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.
>
> After rewording the commit log, I've done a bit of reordering, it's now:
>
>
> commit bce1a8dd0ecf9dfdf3ed1c70da183da618dadb12
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Thu Dec 6 22:46:35 2012 +0100
>
>     Add OsinfoInstallConfig:config-params property
>
>     This property lists the parameters that can be set for a given
>     OsinfoInstallConfig. This is not enforced, it's only there for
>     informative purpose. This will also be used in later commits
>     in order to automatically apply transformations on values
>     for parameters which have an associated OsinfoDatamap.
>
> commit 983f15d436e6fcc41c2db9749c1ebc43367463a7
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Fri Dec 7 12:31:39 2012 +0100
>
>     OsinfoInstallConfig: Use config-params if set
>
>     Now that OsinfoInstallConfig has a 'config-params' property
>     which describes the config parameters when it's set, we can
>     use it when it's available. OsinfoInstallConfigParams can indeed
>     contain a datamap to be used to translate generic libosinfo values
>     to OS-specific values.
>     This commit introduces an osinfo_install_config_get_param_value_list
>     method that will be used in subsequent commits to get these
>     OS-specific values when generating install scripts.
>
> commit bb5a22a02b606026bf97f6d89c4f088e76c70faa
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Thu Dec 6 21:18:45 2012 +0100
>
>     Add osinfo_install_config_new_for_script
>
>     In order to be able to automatically transform configuration parameters
> set
>     on OsinfoInstallConfig instances (ie translate from generic value to
>     OS-specific value), we need to have access to the
>     OsinfoInstallConfigParamList for the current OsinfoInstallScript as
>     this is where the OsinfoDatamaps needed to do the remapping are stored.
>
>     After the previous commits we can now associate an
>     OsinfoInstallConfigParamList with an OsinfoInstallConfig instance,
>     and we can get the OsinfoInstallConfigParamList corresponding to
>     an OsinfoInstallScript instance, so this commit just adds a new
>     osinfo_install_config_new_for_script which will create a new
>     OsinfoInstallConfig instance associated with the
>     OsinfoInstallConfigParamList for a given installation script.
>
> commit d663f67b7ece1d5823e2e2a11f886281d5c7cb2c
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Fri Dec 14 19:03:31 2012 +0100
>
>     Set OsinfoInstallConfig:config-params in
> osinfo_install_script_apply_template
>
>     This allows translation of generic config to OS-specific config if
>     the install script XML specified some datamaps to transform these
>     parameters.
>
> commit b9591db34db3eea540dc28b132fb8a53885e3e05
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Mon Dec 17 20:47:34 2012 +0100
>
>     Use OS-specific config in OsinfoInstallScript
>
>     When generating the unattended installation script, we can now
>     use osinfo_install_config_get_param_list() to get OS-specific
>     values when available. This will only work when an
>     OsinfoInstallConfigParamList is associated with the
>     OsinfoInstallConfig being processed.
>
> (see http://cgit.freedesktop.org/~teuf/libosinfo/log/?h=datamap)
>
> Does it look better this way?

Yeah, it looks good. Not that it wasn't good already. Only the commit
log needed some adjusting and I didn't know that one of the needed
change is already in git master.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124