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

Add Delete/List command for Stage #484

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Add Delete/List command for Stage #484

merged 3 commits into from
Aug 2, 2023

Conversation

devholic
Copy link
Member

This PR adds Delete/List command for Stage to CLI.

@netlify
Copy link

netlify bot commented Jul 24, 2023

Deploy Preview for docs-kargo-akuity-io canceled.

Name Link
🔨 Latest commit 5159f1f
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/64c9b8a0bbdb9600084b7fd7

@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #484 (5159f1f) into main (38bc48d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #484   +/-   ##
=======================================
  Coverage   49.49%   49.49%           
=======================================
  Files          58       58           
  Lines        4271     4271           
=======================================
  Hits         2114     2114           
  Misses       2070     2070           
  Partials       87       87           

Signed-off-by: Sunghoon Kang <[email protected]>
@krancour
Copy link
Member

krancour commented Aug 1, 2023

Implementation looks fine, but I have a concern about the UX...

In #489, for PromotionPolicies, I see a get command that, as with kubectl, pulls double-duty as list. If a name is specified, it's a get and if a name is not specified, it's a list.

Here I see list added and get is conspicuously absent.

I think we need to pick one style and apply it consistently:

Either get and list are always separate commands OR get is implicitly a list command when no name is specified.

@devholic
Copy link
Member Author

devholic commented Aug 2, 2023

@krancour I created an issue for this: #488

I'll merge list command to get to work same as kubectl in another PR.

Copy link
Member

@krancour krancour left a comment

Choose a reason for hiding this comment

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

LGTM assuming #488 will address the minor concerns that were raised.

@devholic devholic merged commit ebacf7e into main Aug 2, 2023
14 checks passed
@devholic devholic deleted the hoon/stage-cli branch August 2, 2023 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants