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

[bug] CLI Help when some "options" are required is self-contradicting #1699

Closed
shannonwells opened this issue Jan 25, 2019 · 5 comments
Closed
Labels
C-bug Category: This is a bug good first issue Call for participation: Well-suited to new project contributors

Comments

@shannonwells
Copy link
Contributor

shannonwells commented Jan 25, 2019

Description

In several places in the CLI help, we have text that puts --someOption in square brackets, but such an option is required. This is confusing, since the extended help has no indication that these things are required. Example is the miner create help text quoted below.

If I run the miner create command as I have before (go-filecoin miner create 10 100), it fails, saying only that --price is required. But both gas price and gas limit are required arguments. In Unix help, square brackets around an argument mean it is optional, however these arguments are in square brackets in the help text. In the "USAGE" section and the miner --help output, --price and --limit aren't shown at all (so the miner command help can't be used as a quick reference because it's incomplete). In the "OPTIONS" shown below, --price and --limit also aren't marked required.

Expected Behavior

All help text should show all required arguments. The only arguments in square brackets should be optional ones. In the extended help (e.g. OPTIONS section), these arguments should say "required" in some way, and be the same way everywhere.

Current Behavior

 $ gf miner create --help
USAGE
  go-filecoin miner create <pledge> <collateral> - Create a new file miner with <pledge> 1GB sectors and <collateral> FIL

SYNOPSIS
  go-filecoin miner create [--from=<from>] [--peerid=<peerid>] [--price=<price>] [--limit=<limit>] [--] <pledge> <collateral>

ARGUMENTS

  <pledge>     - The size of the pledge (in 1GB sectors) for the miner
  <collateral> - The amount of collateral in FIL to be sent (minimum 0.001 FIL per sector)

OPTIONS

  --from   string - Address to send from.
  --peerid string - Base58-encoded libp2p peer ID that the miner will operate.
  --price  string - Price (FIL e.g. 0.00013) to pay for each GasUnits consumed mining this message.
  --limit  uint64 - Maximum number of GasUnits this message is allowed to consume.

Starting points

If there is a single default gas limit and gas price, then the default option text will need to be updated in main.go.

Also look in commands/ for functions that call parseGasOptions:

@shannonwells shannonwells added C-bug Category: This is a bug candidate labels Jan 25, 2019
@mishmosh mishmosh added the good first issue Call for participation: Well-suited to new project contributors label Jan 28, 2019
@mishmosh
Copy link
Contributor

@shannonwells in order to prepare this as a good first issue, can you please add some clarification?

  1. Please specify which command(s) need to be edited
  2. Add links to code

@travisperson
Copy link
Contributor

I believe these are actually going to be optional at some point right? I believe I remember @acruikshank saying that later the price and limit will be set to best guesses automatically.

Should the fix here be to set defaults on the options that can later be removed when we have the best-guess behavior?

@shannonwells
Copy link
Contributor Author

I think that's a good idea. If there are defaults set they should also be documented.

@travisperson
Copy link
Contributor

I think we get the documentation for free (at least if we are just documenting in the command help text). An example of this is the ping command.

https://github.com/filecoin-project/go-filecoin/blob/7cc6a6b790c3fd74e2f68c061a5e239348037b32/commands/ping.go#L27-L29

USAGE
  go-filecoin ping <peer ID>... - Send echo request packets to p2p network members

SYNOPSIS
  go-filecoin ping [--count=<count> | -n] [--] <peer ID>...

ARGUMENTS

  <peer ID>... - ID of peer to be pinged.

OPTIONS

  -n, --count uint - Number of ping messages to send. Default: 10.

@shannonwells
Copy link
Contributor Author

by "documented" I meant "documented in the help output," not in a separate document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug good first issue Call for participation: Well-suited to new project contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants