Skip to content

Remove the ShowDescription flag#13445

Closed
LalatenduMohanty wants to merge 1 commit intohelm:mainfrom
LalatenduMohanty:remove_ShowDescription_helm4_todo
Closed

Remove the ShowDescription flag#13445
LalatenduMohanty wants to merge 1 commit intohelm:mainfrom
LalatenduMohanty:remove_ShowDescription_helm4_todo

Conversation

@LalatenduMohanty
Copy link
Copy Markdown

@LalatenduMohanty LalatenduMohanty commented Nov 14, 2024

What this PR does / why we need it:
It is one of the TODO Helm 4:

Special notes for your reviewer:

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@LalatenduMohanty
Remove the ShowDescription flag
926de91 
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 14, 2024
@LalatenduMohanty LalatenduMohanty mentioned this pull request Nov 14, 2024
Merged
3 tasks
@LalatenduMohanty LalatenduMohanty force-pushed the remove_ShowDescription_helm4_todo branch from 61267fe to 926de91 Compare November 14, 2024 23:07
@robertsirc
Copy link
Copy Markdown
Member

robertsirc commented Nov 15, 2024

Hello, thank you for your PR, does this close an issue?

@LalatenduMohanty
Copy link
Copy Markdown
Author

LalatenduMohanty commented Nov 18, 2024

@robertsirc This fixes the following TODO in the code

// If true, display description to output format,
// only affect print type table.
// TODO Helm 4: Remove this flag and output the description by default.

@LalatenduMohanty
Copy link
Copy Markdown
Author

LalatenduMohanty commented Nov 18, 2024

Also I worked on the PR in the Kubecon NA 24 contribfest. cc @sabre1041

gjenkins8
gjenkins8 approved these changes Nov 19, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@gjenkins8 gjenkins8 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! (agree this should be cleaned up / removed for helm 4)

@gjenkins8 gjenkins8 added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Nov 19, 2024
@sabre1041
Copy link
Copy Markdown
Contributor

sabre1041 commented Nov 19, 2024

@LalatenduMohanty According to the TODO, the goal is to remove the flag, but display description by default. The flag is being removed, but the description was also removed

@LalatenduMohanty
Copy link
Copy Markdown
Author

LalatenduMohanty commented Nov 19, 2024

Wondering if I should fix the PR because #13444 is fixing the same thing, also has the labels and going to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gjenkins8 gjenkins8 gjenkins8 approved these changes

Assignees

No one assigned

Labels

Has One Approval This PR has one approval. It still needs a second approval to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LalatenduMohanty @robertsirc @sabre1041 @gjenkins8