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

adding color configuration from static to yaml #10

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

petryeric
Copy link
Contributor

I changed the static color configuration (as mentioned in the issue https://github.com/KonstantinGasser/scotty/issues/2 to a color config which loads a "config.yaml".

@petryeric petryeric mentioned this pull request Mar 27, 2023
Copy link
Owner

@KonstantinGasser KonstantinGasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it LGTM.
Let's debate about one thing which is the location of the config file. Based on your current implementation the config.yaml would need to be located where ever the scotty command is called, do I understand that correctly?
Maybe we should aim for a common place say $HOME/.scotty/config.yaml, what are your thoughts?

Also we should fallback to default values in case no config exits, so that we do not force the user to have a config

@petryeric
Copy link
Contributor Author

Yea that's a good point, would be way more useful for the enduser to have their config and one central place
didn't think of it that way

Copy link
Owner

@KonstantinGasser KonstantinGasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; Will be included in the v0.1.0 release 😄

@petryeric petryeric closed this Mar 30, 2023
@KonstantinGasser KonstantinGasser merged commit 89a4d79 into KonstantinGasser:main Apr 7, 2023
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

2 participants