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
