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

converts config to yaml #62

Closed
wants to merge 1 commit into from

Conversation

jshuping
Copy link

@jshuping jshuping commented Aug 8, 2024

For Issue #58 , didn't realize it was already assigned until after I made the changes, whoops!

@kevinelliott
Copy link
Collaborator

kevinelliott commented Aug 22, 2024

@jshuping Thanks! This looks great. The only reason I haven't merged this yet is because I'm thinking through whether or not YAML or ini would be better for being littered with comments describing the configuration.

Putting comments in YAML is easy enough, but I wonder if all the tabbing would get lost after a few paragraphs of descriptions, etc.

What are your thoughts?

@SimmerV
Copy link
Contributor

SimmerV commented Aug 23, 2024

I think .ini structure is slightly easier than .yaml but either is better than .json

@kevinelliott
Copy link
Collaborator

We chatted in the Discord about this:

just a snippet:

YAML
Pros:
concise
indentation makes groups/sections relatively visible
options are condensed together
Cons:
indentation hard for less technical users (oops 3 spaces instead of 2, didn't see that)
indentation less obvious once there is a lot of documentation

TOML
Pros:
familiar (at least to some who have worked with config files)
explicit sections to group like config options within
documentation feels natural here, can document a section, a subsection, and individual options without impact to a parsed layout
Cons: 
a bit more verbose
lack of visual heirarchy

From my developer perspective, JSON and YAML are the clear winners. From a user perspective (who are assumed less technical than us), I think TOML is likely the winner.

@kevinelliott
Copy link
Collaborator

Alright, we have decided to use ini.

@jshuping Would you be interested in converting this PR to using ini?

@kevinelliott
Copy link
Collaborator

@jshuping calling once... calling twice...

@jshuping
Copy link
Author

jshuping commented Sep 9, 2024

HI @kevinelliott I'll let you or someone else take it on, I won't be able to help the next few weeks, but will look forward to contributing in the future!

@jshuping jshuping closed this Sep 9, 2024
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.

3 participants