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

Revise help text for pool metadata source CLI option #2241

Merged
merged 4 commits into from
Oct 16, 2020

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Oct 14, 2020

Issue Number

ADP-482

Overview

Noticed while reviewing IntersectMBO/cardano-launcher#90.

  • Fix variable names to reflect the new option
  • Add more detail to CLI help text

Noticed while reviewing IntersectMBO/cardano-launcher#90.

- Fix variable names to reflect the new option
- Add more detail to CLI help text
@rvl rvl added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Oct 14, 2020
@rvl rvl self-assigned this Oct 14, 2020
<> "Provide a URL to specify a SMASH metadata proxy server, "
<> "use \"direct\" to fetch directly from the registered pool URLs,"
<> " or \"none\" to completely disable stake pool metadata. "
<> "This setting will persist across restarts.")
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps: "This setting will persist across restarts AND override any previous setting set via this option or via the API." ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to think it was a bad idea to keep the cli switch. It's confusing. Now someone might still think it pins the setting or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fine to have a sticky setting like this. Makes sense to me. Anyway, our users are likely to either configure this by the CLI, or by API, but not both.

Reworded a bit more.

@KtorZ
Copy link
Member

KtorZ commented Oct 16, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 16, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit 0640a19 into master Oct 16, 2020
@iohk-bors iohk-bors bot deleted the rvl/adp-482/pool-metadata-source-cli branch October 16, 2020 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants