Skip to content

fix(hosting): mark app-type and hosting-product as required in help - #345

Open
jhenderson1-godaddy wants to merge 1 commit into
mainfrom
fix/hosting-show-sample-values-req-fields
Open

jhenderson1-godaddy wants to merge 1 commit into
mainfrom
fix/hosting-show-sample-values-req-fields

Conversation

@jhenderson1-godaddy

@jhenderson1-godaddy jhenderson1-godaddy commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

--app-type and --hosting-product are required, but --help listed them like optional flags. This labels them "(required)" in their help entries and adds a test so the label stays in place.

Why

The API requires appType and hostingProduct, and the CLI rejects a missing value. The usage line shows the flag outside [OPTIONS], but the options block gave no sign of it. --app-type was also described as "Hosting product", which reads like --hosting-product on the other command.

Changes

  • --app-type on hosting app list and hosting app create, and --hosting-product on hosting subscription list, now read "(required)" in --help.
  • --app-type is described as the application type. The hosting app group help says "application type" too.
  • The hand-written "is required" sentences are removed from the app list and subscription list descriptions. The value list copied by hand into --hosting-product is removed as well, since clap prints the possible values itself.
  • A new test runs app list, app create and subscription list without the flag. It checks that each fails and that its --help entry says "(required)". The CLI helpers it uses moved from the app create tests into hosting/test_support.rs.

Not in this PR

Other feedback suggested two things, and neither of them is included here.

  • Listing the allowed values when a required flag is missing. Clap builds that error and cli-engine prints it as is, so this needs a change in cli-engine. An invalid value already lists the options.
  • Dropping the flag, or defaulting it, when only one value is eligible. MHWP is behind a feature flag. Once it is on, hosting app list with no flag would either fail or default to NODEJS and hide WordPress apps from the list, so I kept the flag required. This one is open for discussion.

Tested

Run on this branch:

  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • check-module-size.sh

…Label both flags '(required)' in --help and add a test that pins it.
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.

1 participant