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

chore(daemon): removing password, entropy and validator number as prompt, and adding flags instead #943

Merged
merged 5 commits into from
Jan 18, 2024

Conversation

kehiy
Copy link
Contributor

@kehiy kehiy commented Jan 17, 2024

Description

In this PR I removed the validator number, password, and entropy as prompt and hard-coded values. now we get them as flags from the user, this helps other interfaces and clients like GUI use this init command with ease.

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Merging #943 (8133303) into main (1aa034c) will increase coverage by 0.02%.
The diff coverage is n/a.

❗ Current head 8133303 differs from pull request most recent head 3144fbc. Consider uploading reports for the commit 3144fbc to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
+ Coverage   83.42%   83.45%   +0.02%     
==========================================
  Files         171      171              
  Lines        8840     8840              
==========================================
+ Hits         7375     7377       +2     
+ Misses       1111     1109       -2     
  Partials      354      354              

cmd/daemon/init.go Outdated Show resolved Hide resolved
@b00f
Copy link
Collaborator

b00f commented Jan 17, 2024

After looking at all flags I suggest change the text to this:

workingDirOpt := initCmd.Flags().StringP("working-dir", "w",
		cmd.PactusDefaultHomeDir(), "the path to the working directory to save the wallet and node files")

	testnetOpt := initCmd.Flags().Bool("testnet",
		true, "initialize working directory for joining the testnet") // TODO: make it false after mainnet launch

	localnetOpt := initCmd.Flags().Bool("localnet",
		false, "initialize working directory for localnet for developers")

	restoreOpt := initCmd.Flags().String("restore",
		"", "restore the 'default_wallet' using a mnemonic or seed phrase")

	passwordOpt := initCmd.Flags().StringP("password", "p",
		"", "the wallet password")

	entropyOpt := initCmd.Flags().IntP("entropy", "e",
		128, "entropy bits for seed generation. range: 128 to 256")

	valNumOpt := initCmd.Flags().Int("val-num",
		0, "number of validators to be created. range: 1 to 32")
		```
		
Note: 
- all messages start with lowercase
- No default in the help string (Cobra adds default)
- Add range for flags that accepts range
- No parentheses "()" in help message

@kehiy kehiy requested a review from b00f January 17, 2024 15:30
@kehiy kehiy requested a review from b00f January 17, 2024 15:44
@kehiy
Copy link
Contributor Author

kehiy commented Jan 17, 2024

@b00f fixed.
should we also do the same checks for passwordOpt?

@b00f b00f merged commit 9531bd0 into pactus-project:main Jan 18, 2024
10 checks passed
@kehiy kehiy deleted the feature/daemonUpdate branch January 22, 2024 19:10
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.

2 participants