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

Various improvements to config code #102

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mfdorst
Copy link
Contributor

@mfdorst mfdorst commented Apr 19, 2024

  • Make error messages more informative
  • Add doc comments
  • Make generate_config() private
  • Reorder code so that public stuff comes first
  • Fix typo

@HackedOS
Copy link
Member

We were actually thinking of moving away from RON to a more user-friendly custom config, this might be an opportunity

@mfdorst
Copy link
Contributor Author

mfdorst commented Apr 19, 2024

Oh awesome, I was gonna ask if I could do that. I was thinking TOML, is that what you had in mind?

@mfdorst
Copy link
Contributor Author

mfdorst commented Apr 19, 2024

I'm also considering storing the default config as TOML or whatever we go with so that we don't have to write out the default config in rust, serialize it, then immediately de-serialize it the way we're doing now.

@mfdorst
Copy link
Contributor Author

mfdorst commented Apr 19, 2024

As per our discord discussion, TOML is off the table for this PR.

@eepy-goddess
Copy link
Member

I hope we don't pick yaml :p

@mfdorst
Copy link
Contributor Author

mfdorst commented Apr 19, 2024

When I say TOML is off the table I mean this PR is done, I am not implementing a config language change at this time.

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.

None yet

3 participants