argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

chore: improve argocd app delete

Open my-git9 opened this issue 2 years ago • 3 comments

A small thing, I feel that the confirmation of 'y/n' is placed on the same line as the output to make people feel used to it, it's just a personal feeling

Checklist:

  • [ ] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • [ ] The title of the PR states what changed and the related issues number (used for the release note).
  • [ ] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • [ ] Does this PR require documentation updates?
  • [ ] I've updated documentation as required by this PR.
  • [ ] Optional. My organization is added to USERS.md.
  • [x] I have signed off all my commits as required by DCO
  • [ ] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • [ ] My build is green (troubleshooting builds).

my-git9 avatar Aug 01 '22 15:08 my-git9

Codecov Report

Merging #10160 (3fa1da5) into master (33f1759) will increase coverage by 0.06%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #10160      +/-   ##
==========================================
+ Coverage   45.92%   45.99%   +0.06%     
==========================================
  Files         227      226       -1     
  Lines       27403    27408       +5     
==========================================
+ Hits        12584    12605      +21     
+ Misses      13110    13089      -21     
- Partials     1709     1714       +5     
Impacted Files Coverage Δ
cmd/argocd/commands/app.go 20.51% <0.00%> (+0.13%) :arrow_up:
util/git/creds.go 40.64% <0.00%> (-0.54%) :arrow_down:
controller/state.go 73.89% <0.00%> (-0.33%) :arrow_down:
util/helm/cmd.go 25.00% <0.00%> (-0.30%) :arrow_down:
controller/sync.go 56.09% <0.00%> (ø)
cmd/util/cluster.go 41.17% <0.00%> (ø)
pkg/apiclient/apiclient.go 0.96% <0.00%> (ø)
reposerver/repository/repository.go 64.30% <0.00%> (ø)
cmd/argocd/commands/admin/settings_rbac.go 23.80% <0.00%> (ø)
...is/applicationset/v1alpha1/applicationset_types.go 34.69% <0.00%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 01 '22 16:08 codecov[bot]

On this note, I think a real improvement would be to refactor this code to use https://github.com/argoproj/argo-cd/blob/7bd9e1eb83df515f1f558b2a524ffb5f8b2e935a/util/cli/cli.go#L115 :)

jannfis avatar Aug 01 '22 17:08 jannfis

On this note, I think a real improvement would be to refactor this code to use

https://github.com/argoproj/argo-cd/blob/7bd9e1eb83df515f1f558b2a524ffb5f8b2e935a/util/cli/cli.go#L115

:)

There is an option of 'A' for confirmAnswer, which seems to be not so optimized: https://github.com/argoproj/argo-cd/blob/master/cmd/argocd/commands/app.go#L1104

my-git9 avatar Aug 02 '22 00:08 my-git9

On this note, I think a real improvement would be to refactor this code to use

https://github.com/argoproj/argo-cd/blob/7bd9e1eb83df515f1f558b2a524ffb5f8b2e935a/util/cli/cli.go#L115

:)

Hi @jannfis , according to your suggestion, I made some changes to this PR, can you help me to review it? Thanks

my-git9 avatar Aug 07 '22 09:08 my-git9