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

config: Deprecate --configfile. #470

Merged
merged 1 commit into from
May 21, 2024
Merged

Conversation

jholdstock
Copy link
Member

This removes the --configfile option from vspd. It introduced quite a few weird edge cases (and a fair bit of code to deal with them) but afaik nobody actually used it. Note that the --homedir option stays, so it is still possible to run vspd with config in a non-default location if required.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

This isn't my project, but I personally disagree with this. I use config file overrides all the time with shell aliases to do testing.

This removes the --configfile option from vspd. It introduced quite a
few weird edge cases (and a fair bit of code to deal with them) but
afaik nobody actually used it. Note that the --homedir option stays, so
it is still possible to run vspd with config in a non-default location
if required.
@jholdstock
Copy link
Member Author

This feature was incidentally inherited from dcrd a few years ago via copy & pasting config.go and it hasn't been used/tested/maintained since. I understand this feature could be more important in dcrd where specifying a different config file whilst reusing an existing datadir would be desirable, but vspd does not have the same restriction of a multi-gigabyte datadir.

I'd like to drop this behavior so that config creation/management can be more easily refactored into a new purpose built binary (#465). Preserving the behavior while nobody uses it creates extra work unnecessarily.

Every config value can still be overwritten via CLI, so testing with shell aliases is still feasible.

@davecgh
Copy link
Member

davecgh commented May 19, 2024

Code looks fine, so I'll approve. I still disagree with the premise though. I know it's possible to use a shell alias to specify a bunch of command line options, but it is far more annoying than having separate config files that the aliases point to. You can modify the config file without having to mess with updating the shell aliases and restarting the shell or rehashing, etc. I also keep the config files in version control so it's easy to compare them and even go back to older versions of software where the options were different. Config files are generally just a better choice when it comes to managing config.

@jholdstock jholdstock merged commit 8b6b2e4 into decred:master May 21, 2024
2 checks passed
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