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

adding new column message in kctrl pkg installed list command #1475

Merged

Conversation

prembhaskal
Copy link
Member

@prembhaskal prembhaskal commented Feb 6, 2024

What this PR does / why we need it:

Adding a new column 'Message' in the output of command kctrl package installed list, which will show the message from last condition from the status of each package install when --wide flag is provided.

The sample output from testing.

 % /tmp/kctrl package installed list
Target cluster 'https://127.0.0.1:50874' (nodes: local-test-control-plane)

Installed packages in namespace 'default'

Name          Package Name         Package Version  Status  
pkg-demo      simple-app.corp.com  1.0.0            Reconcile succeeded  
pkg-demo-err  simple-app.corp.com  1.0.0            Reconcile failed  

Succeeded
 % /tmp/kctrl package installed list --wide
Target cluster 'https://127.0.0.1:50874' (nodes: local-test-control-plane)

Installed packages in namespace 'default'

Name          Package Name         Package Version  Status               Message  
pkg-demo      simple-app.corp.com  1.0.0            Reconcile succeeded  -  
pkg-demo-err  simple-app.corp.com  1.0.0            Reconcile failed     Error (see .status.usefulErrorMessage for details)  

Succeeded


Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?

Adds a new column 'Message' in the output of command `kctrl package installed list`, which will show the message from last condition from the status of each package install when --wide flag is provided

Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


@prembhaskal prembhaskal requested review from 100mik and praveenrewar and removed request for 100mik February 6, 2024 08:34
@prembhaskal prembhaskal marked this pull request as ready for review February 6, 2024 08:36
@prembhaskal prembhaskal force-pushed the kctrl-list-new-msg-column branch from 371e222 to 4ab281b Compare February 6, 2024 09:06
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

Do you think we should keep the column hidden by default and only display it when any one of the pkgi is failing?
(I am okay if we want to think about that later on based on feedback from users)

cli/test/e2e/package_install_test.go Outdated Show resolved Hide resolved
@prembhaskal prembhaskal force-pushed the kctrl-list-new-msg-column branch from 4ab281b to c84c585 Compare February 6, 2024 10:05
@prembhaskal prembhaskal force-pushed the kctrl-list-new-msg-column branch from c84c585 to 3e6016e Compare February 7, 2024 12:04
@prembhaskal
Copy link
Member Author

Do you think we should keep the column hidden by default and only display it when any one of the pkgi is failing? (I am okay if we want to think about that later on based on feedback from users)

so moved the new column under --wide flag.

Copy link
Contributor

@100mik 100mik left a comment

Choose a reason for hiding this comment

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

LGTM!

@prembhaskal prembhaskal force-pushed the kctrl-list-new-msg-column branch 2 times, most recently from b8889b2 to b04992a Compare February 8, 2024 11:45
column will be visible when --wide flag is set to true

Signed-off-by: Premkumar Bhaskal <[email protected]>
@prembhaskal prembhaskal force-pushed the kctrl-list-new-msg-column branch from b04992a to e99b02f Compare February 8, 2024 12:16
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

LGTM!

@prembhaskal prembhaskal merged commit 87059e5 into carvel-dev:develop Feb 8, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants