Re: [PATCH 0/6] Simplify application of datamaps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Исходное сообщение -----
> 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

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux