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

Update help text for commands + params in CLI and top-level API #7176

Merged
merged 32 commits into from
Apr 11, 2023

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented Mar 16, 2023

resolves #6546

Description

See #6546 for description.

Removals

  • dbt parse --compile/--no-compile
  • dbt parse --write-manifest
  • dbt compile --parse-only
  • dbt show --parse-only

Additions

Added these three as synonyms per suggestion "12-factor CLI" and CLIG:

  • --version
  • -V
  • -v

Same thing here:

  • --quiet
  • -q

By explicitly attaching -V, -v, and -q to their most conventional uses, we can proactively prevent them from accidentally being attached in a non-conventional way.

Help text reviewed

Reviewed the output of:

  • dbt build --help
  • dbt clean --help
  • dbt compile --help
  • dbt debug --help
  • dbt deps --help
  • dbt docs --help
  • dbt init --help
  • dbt list --help
  • dbt parse --help
  • dbt run --help
  • dbt run-operation --help
  • dbt seed --help
  • dbt snapshot --help
  • dbt source --help
  • dbt test --help

Testing

  • ⚠️ dbt debug --config-dir doesn't appear in any functional tests

Side notes

CLIG suggests:

  • flags like --json and --plain to specify how output is rendered to standard out
  • standard usage of -o, --output is to specify the path of the output file

However, we are using --output json and --output text instead of --json and --plain in multiple sub-commands. The one exception is dbt source freshness --output PATH which does accept an output file path.

I'm not sure how much pain or annoyance any of this is causing though.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
  • I have run changie new to create a changelog entry
  • I have reviewed the following TODO's:
    • git grep "TODO" core/dbt/cli/main.py
    • git grep "TODO" core/dbt/cli/params.py

@cla-bot cla-bot bot added the cla:yes label Mar 16, 2023
@dbeatty10 dbeatty10 added the Skip Changelog Skips GHA to check for changelog file label Mar 16, 2023
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 7, 2023

@dbeatty10 Is this nearly ready for review? Even if you didn't get 100% through all commands/params, I'd love to include what you've already got, before we cut v1.5.0-rc1 next week!

@dbeatty10
Copy link
Contributor Author

@dbeatty10 Is this nearly ready for review? Even if you didn't get 100% through all commands/params, I'd love to include what you've already got, before we cut v1.5.0-rc1 next week!

I can put this up for review later today 👍

@aranke
Copy link
Member

aranke commented Apr 7, 2023

Added these three as synonyms per suggestion "12-factor CLI" and CLIG

Love to see this, thanks @dbeatty10!

@dbeatty10 dbeatty10 marked this pull request as ready for review April 7, 2023 21:59
@dbeatty10 dbeatty10 requested a review from a team as a code owner April 7, 2023 21:59
@dbeatty10 dbeatty10 merged commit 828d723 into main Apr 11, 2023
@dbeatty10 dbeatty10 deleted the dbeatty/6546-param-help-text branch April 11, 2023 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes Skip Changelog Skips GHA to check for changelog file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-1766] [DX] Review command + param help text in CLI / top-level API
5 participants