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
feat: Move app resources commands to dedicated command file #9306
feat: Move app resources commands to dedicated command file #9306
Conversation
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: pashavictorovich <pavel@codefresh.io> # Conflicts: # OWNERS
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: pashavictorovich <pavel@codefresh.io>
…e_cli_distinguish � Conflicts: � cmd/argocd/commands/app_test.go
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Codecov Report
@@ Coverage Diff @@
## master #9306 +/- ##
==========================================
+ Coverage 46.06% 46.08% +0.02%
==========================================
Files 217 218 +1
Lines 25908 25917 +9
==========================================
+ Hits 11934 11945 +11
+ Misses 12317 12316 -1
+ Partials 1657 1656 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's just a cleanup PR, but I'd love to see unit tests on the util functions. 😄
If there's no time for that, LGTM besides one nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great refactoring PR! Tks for this.
I think the main purpose of this refactoring is to enable proper unit testing the command. It would be great to have at least the main test cases implemented as part of this PR.
Signed-off-by: pashavictorovich <pavel@codefresh.io>
@leoluz @crenshaw-dev |
Signed-off-by: pashavictorovich <pavel@codefresh.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks Pasha!
Move commands to dedicated file, also few functions to utils
NewApplicationPatchResourceCommand
NewApplicationDeleteResourceCommand
NewApplicationListResourceCommand