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 Default Config Settings #1245

Merged
merged 7 commits into from
Jul 19, 2024
Merged

Add Default Config Settings #1245

merged 7 commits into from
Jul 19, 2024

Conversation

shellvish
Copy link
Contributor

Heavily inspired by Osmosis default config.toml+app.toml settings.

Copy link
Collaborator

@sampocs sampocs left a comment

Choose a reason for hiding this comment

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

lgtm. although, can you point out what was different in this vs the osmosis version / anything you had to change yourself?

//
// Silently handles and skips any error/panic due to write permission issues.
// No-op otherwise.
func overwriteConfigTomlValues(serverCtx *server.Context) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: are the config and app toml functions identical besides the reference to "config.toml/app.toml" and "recommendedConfigTomlValues/recommendedAppTomlValues"? If so, might make sense to consolidate the functions

@sampocs sampocs added A:automerge Automatically merge PR once checks pass and removed A:automerge Automatically merge PR once checks pass labels Jul 19, 2024
@sampocs sampocs added A:automerge Automatically merge PR once checks pass and removed A:automerge Automatically merge PR once checks pass labels Jul 19, 2024
@sampocs sampocs merged commit 158e49c into main Jul 19, 2024
10 checks passed
@sampocs sampocs deleted the add-default-config-settings branch July 19, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants