[Libosinfo] [PATCH 0/6] Simplify application of datamaps

Christophe Fergeau cfergeau at redhat.com
Wed Jan 9 16:02:14 UTC 2013



----- Исходное сообщение -----
> This is all complete overkill - the InstallScript class already has
> info about the datamaps and can apply them to the original
> InstallConfig
> object instance it has, without needing to create a throwaway copy.
> 
> This series reverts the last 5 patches in that quoted series and
> applies one simpler patch at the end which does the same job without
> needing to directly associate the InstallConfig & InstallScript
> classes at all.

While having an OsinfoInstallConfigParams property in the OsinfoInstallConfig class, it imo makes sense to have it as the disconnect between OsinfoInstallConfig and OsinfoInstallConfigParams is weird. It's more convenient to have the configuration schema (OsinfoInstallConfigParams) directly available while filling an OsinfoInstallConfig object, and would potentially allow in the future to check which parameters are allowed/disallowed/... when they are set.

Regarding the osinfo_install_script_get_param_value_list code, imo it fits better in OsinfoInstallConfig as a special getter similar to osinfo_entity_get_param_value_list, but with a specialized behaviour.

As for the throw-away copy, I'm also all for getting rid of it, it was added after I got an objection during review. The code was initially doing a straight g_object_set(config, "config-params", params, NULL); during the generation of the install script instead of a copy.

Christophe




More information about the Libosinfo mailing list