Re: [libvirt-ci RFC PATCH 00/13] Introduce a new global TOML config file for lcitool
On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
This series is trying to consolidate the number of config files we
currently
recognize under ~/.config/lcitool to a single global TOML config file. Thanks
to this effort we can expose more seetings than we previously could which will
come handy in terms of generating cloudinit images for OpenStack.
Patches 1-4 patches are just a little extra - not heavily related to the series
See patch 5/13 why TOML was selected.
First of all, thanks for tackling this! It's something that we've
sorely needed for a while now.
Now, I know I'm the one who suggested TOML in the first place... But
looking at the series now I can't help but think, why not YAML? O:-)
Since we're using it extensively already due to Ansible, I think it
would make sense to use it for the configuration file as well. It's
easy enough to consume for a human, and we wouldn't need to drag in
an additional dependency. I also believe, perhaps naively, that
adapting your code to use YAML instead of TOML wouldn't require much
work - from the Python point of view, you basically end up with a
dictionary in both cases.
Feel free to argue against this suggestion! For example, if you agree
with it in principle but feel like it's unfair of me to ask you to
rework your code, I'm happy to port it myself.
As for the rest of the series - I've only skimmed it so far, but I
did not spot anything horribly wrong with it at first glance. I'll
provide an actual, detailed review next week.
--
Andrea Bolognani / Red Hat / Virtualization