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 vspclosedmsg config. #269

Merged
merged 1 commit into from Jun 10, 2021
Merged

Add vspclosedmsg config. #269

merged 1 commit into from Jun 10, 2021

Conversation

jholdstock
Copy link
Member

If vspclosed is set to true, the provided message will be displayed on the webpage and returned by the status API endpoint.

Suggested by @xaur in #232

If vspclosed is set to true, the provided message will be displayed on the webpage and returned by the status API endpoint.
@@ -292,6 +293,11 @@ func loadConfig() (*config, error) {
return nil, errors.New("invalid vspfee - should be greater than 0.01 and less than 100.0")
}

// If VSP is not closed, ignore any provided closure message.
if !cfg.VspClosed {
cfg.VspClosedMsg = ""
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why this needs to be set to nil if there is a flag available which can be checked before the message is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

This same if statement will still need to exist in vspinfo.go to ensure that the message is only included in /vspinfo response when the VSP is actually closed.

I put it in config.go instead because the value of VspClosedMsg should never be used when VspClosed == true, so putting it here will prevent the need for it to be duplicated in other places which might access the closed message in future.

Copy link
Member

Choose a reason for hiding this comment

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

I get that, but ignoring the closure message by setting it to an empty string isn't intuitive to me, if the checking the flag precedes using the closure message then it'd only be displayed if the check passes anyway so why mutate the config value? Plus the check isn't expensive. The current approach works and I do not have a strong opinion on this, take another look however.

webapi/templates/homepage.html Show resolved Hide resolved
@jholdstock jholdstock merged commit d1a838b into decred:master Jun 10, 2021
@jholdstock jholdstock deleted the closedmsg branch January 25, 2022 09:50
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