Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config loader for json and yaml #37

Closed
wants to merge 1 commit into from

Conversation

darkdragon-001
Copy link

This adds config loaders for json and yaml formats in addition to toml format.

I am just not sure which packages to add to setup.py.

@darkdragon-001
Copy link
Author

do you acutely need this?

This could be seen as preparation for #36. For a deeper hierarchy, I personally find yaml or json a lot clearer than toml's [key.sub.sub] syntax.

I mean it's nice and all but it does add extra maintenance for a problem I hadn't had this far.

Python's json and yaml libraries are used in many many applications and can be seen very stable. If these libraries break, then half of the world breaks. Therefore, most system administrators are more used to these formats (I had to search for toml syntax myself). Since it's only these few lines to maintain, I think this shouldn't add too much maintenance work. You can keep the schema and for the documentation documentation keep the toml examples (maybe just add a sentence telling json and yaml are also supported).

@andreasnuesslein
Copy link
Member

Python's json and yaml libraries are used in many many applications and can be seen very stable. If these libraries break, then half of the world breaks.

That's not what I mean. I'm just talking about a general overhead that is introduced by a feature nobody actually needs (it seems).

However I'm also not saying I won't merge it - but could you please add some tests so that when it breaks the tests might let us know about it first? And update the docs a bit to let people know they could also use yaml or json directly

Cheers

@darkdragon-001
Copy link
Author

I'm just talking about a general overhead that is introduced by a feature nobody actually needs (it seems).

As long as something can be achieved somehow, nothing is needed, still could be preferred. Furthermore, how do you come to the conclusion that nobody would like to have such a feature?

However I'm also not saying I won't merge it - but could you please add some tests so that when it breaks the tests might let us know about it first? And update the docs a bit to let people know they could also use yaml or json directly

Okay, I will look for the right places, to add the docs and tests...

@andreasnuesslein andreasnuesslein deleted the branch sinnwerkstatt:master February 15, 2022 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants