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

Minor improvemenets to pk and chainpk test config #1278

Merged
merged 2 commits into from
May 12, 2023
Merged

Minor improvemenets to pk and chainpk test config #1278

merged 2 commits into from
May 12, 2023

Conversation

geoknee
Copy link
Contributor

@geoknee geoknee commented May 12, 2023

No description provided.

geoknee added 2 commits May 12, 2023 10:22
We can't use the Required: true API from "github.com/urfave/cli/v2"
since that will complain even if the flag is set in a config file.

urfave/cli#849

In future we may want to force these keys to be supplied at the command line (best practices would steer away from storing sensitive information in a config file).

This commit also enhances the output of the constructor.
4 unique keys for deployer,alice,bob,irene

also make stderr colorful
@netlify
Copy link

netlify bot commented May 12, 2023

Deploy Preview for nitrodocs canceled.

Name Link
🔨 Latest commit fd3e066
🔍 Latest deploy log https://app.netlify.com/sites/nitrodocs/deploys/645e07737e60150007a6de01

@@ -121,21 +117,29 @@ func main() {
Flags: flags,
Before: altsrc.InitInputSourceWithContext(flags, altsrc.NewTomlSourceFromFlagFunc(CONFIG)),
Action: func(cCtx *cli.Context) error {
if pkString == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we set Required: true when constructing the flag then urfave will handle making sure that a value is provided. (See naaddress)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I tried that (see commit messages).

The problem with doing that is, urfave will insist that the flag is set at the command line -- even if it is set in the config toml file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@github-actions
Copy link

@geoknee geoknee merged commit f6cab5a into main May 12, 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.

2 participants