fix(list): reintroduce deprecated --all/-a flag for backward compatibility#31886
fix(list): reintroduce deprecated --all/-a flag for backward compatibility#31886umut-polat wants to merge 1 commit intohelm:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Reintroduces the legacy --all/-a flag to helm list as a deprecated, backward-compatibility option in Helm v4, where listing “all statuses” is already the default. This addresses scripts broken by removal of the flag (Fixes #31784).
Changes:
- Adds a deprecated
--all/-aflag to thelistcommand and emits a deprecation warning when used. - Adds golden-output coverage for
helm list --allandhelm list -a. - Introduces a new golden file capturing the deprecation warning + list output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cmd/list.go | Reintroduces --all/-a as a deprecated flag on helm list. |
| pkg/cmd/list_test.go | Adds tests asserting output (including deprecation warning) for --all and -a. |
| pkg/cmd/testdata/output/list-all-deprecated.txt | New golden output including the deprecation warning line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := cmd.Flags().MarkDeprecated("all", "this is now the default behavior in Helm v4 and this flag will be removed in a future release"); err != nil { | ||
| panic(err) | ||
| } |
There was a problem hiding this comment.
Avoid panicking on MarkDeprecated errors during command construction. Other commands call FlagSet.MarkDeprecated without error handling; if you want to handle this, prefer returning an error from RunE/PreRunE or keep the consistent non-panicking pattern (e.g., call f.MarkDeprecated("all", ...) and ignore the error). A panic here would crash the CLI if the flag registration ever changes.
| if err := cmd.Flags().MarkDeprecated("all", "this is now the default behavior in Helm v4 and this flag will be removed in a future release"); err != nil { | |
| panic(err) | |
| } | |
| _ = cmd.Flags().MarkDeprecated("all", "this is now the default behavior in Helm v4 and this flag will be removed in a future release") |
|
|
||
| // deprecated flags -- kept for backwards compatibility with helm v3 scripts | ||
| var all bool | ||
| f.BoolVarP(&all, "all", "a", false, "show all releases without any filter applied (default behavior in v4)") |
There was a problem hiding this comment.
The deprecated --all/-a flag is currently bound to a local variable that is never read, which makes the intent (no-op vs behavior-affecting) unclear and easy to regress. Consider binding it to an existing field (e.g., client.All) or at least updating the flag help text to explicitly say it is deprecated and has no effect in v4 (beyond emitting a warning).
| f.BoolVarP(&all, "all", "a", false, "show all releases without any filter applied (default behavior in v4)") | |
| f.BoolVarP(&all, "all", "a", false, "DEPRECATED: has no effect in Helm v4; showing all releases is now the default behavior") |
| name: "list with deprecated --all flag produces same output", | ||
| cmd: "list --all", | ||
| golden: "output/list-all-deprecated.txt", | ||
| rels: releaseFixture, | ||
| }, { | ||
| name: "list with deprecated -a flag produces same output", |
There was a problem hiding this comment.
These test names say the deprecated flag “produces same output”, but the golden output includes an additional deprecation warning line. Renaming the cases to reflect that a deprecation warning is emitted would make the tests clearer (e.g., “emits deprecation warning and lists releases”).
| name: "list with deprecated --all flag produces same output", | |
| cmd: "list --all", | |
| golden: "output/list-all-deprecated.txt", | |
| rels: releaseFixture, | |
| }, { | |
| name: "list with deprecated -a flag produces same output", | |
| name: "list with deprecated --all flag emits deprecation warning and lists releases", | |
| cmd: "list --all", | |
| golden: "output/list-all-deprecated.txt", | |
| rels: releaseFixture, | |
| }, { | |
| name: "list with deprecated -a flag emits deprecation warning and lists releases", |
…ility In Helm v4, 'helm list' shows all releases by default, making the --all/-a flag unnecessary. However, removing the flag entirely was a breaking change that broke existing scripts and automation. This reintroduces --all/-a as a deprecated no-op flag that emits a warning, easing the transition for users upgrading from Helm v3. Fixes helm#31784 Signed-off-by: Umut Polat <52835619+umut-polat@users.noreply.github.com>
325a724 to
795534f
Compare
Reintroduces --all/-a for helm list as a deprecated no-op with a warning. In v4, helm list shows all releases by default, making this flag unnecessary - but removing it broke existing scripts. Follows the same MarkDeprecated pattern used for --force and --atomic.
Fixes #31784