Improves odo delete for devfiles #2842
Conversation
openshift-ci-robot
added
kind/bug
openshift-ci-robot
added
the
lgtm
|
prow issue |
| @@ -210,6 +181,44 @@ func (do *DeleteOptions) Run() (err error) { | |||
| return | |||
| } | |||
|
|
|||
| // Run has the logic to perform the required actions as part of command for devfiles | |||
| func (do *DeleteOptions) DevFileRun() (err error) { | |||
There was a problem hiding this comment.
@mik-dass Any plan for writing UTs ?
There was a problem hiding this comment.
It's the CLI package. We are testing CLI by using integration tests I guess.
There was a problem hiding this comment.
Really i am in 50-50 state whether to write UTs for cli packages or not. Recently i saw UTs for few devfile cli packages, so thought why not then covering UTs for regular cli packages. More importantly our UTs coverage are being run on all regular cli packages having no test file.
Ping @girishramnani @kadel
There was a problem hiding this comment.
Recently i saw UTs for few devfile cli packages, so thought why not then covering UTs for regular cli packages.
TBH I couldn't any. Please share some links. Also is the effort even worth it?
There was a problem hiding this comment.
This is a generic topic for cabal discussion i guess. I will create a separate issue #2867 to tack the discussion of whether to add UTs for cli packages are sensible or not.
So atm i am applying lgtm label for your current changes
/lgtm
There was a problem hiding this comment.
Cool thanks for creating the issue 👍
There was a problem hiding this comment.
TBH I couldn't any. Please share some links. Also is the effort even worth it?
Please refer #2867, there i have added the references.
|
--now flag flake |
Codecov Report
@@ Coverage Diff @@
## master #2842 +/- ##
==========================================
+ Coverage 43.70% 43.81% +0.10%
==========================================
Files 102 102
Lines 9160 9160
==========================================
+ Hits 4003 4013 +10
+ Misses 4780 4774 -6
+ Partials 377 373 -4
Continue to review full report at Codecov.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kadel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
8 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
| return err | ||
| } | ||
| err = do.EnvSpecificInfo.DeleteEnvDirIfEmpty() | ||
| if err != nil { |
There was a problem hiding this comment.
IMO, we should remove .odo folder also for odo delete --all, as --all signifies to delete all odo related data.
mik-dass
added
the
do-not-merge/hold
mik-dass
force-pushed
the
dev_del_improve
branch
from
9337a7a
to
4299c56
Compare
openshift-ci-robot
removed
the
lgtm
|
/lgtm |
openshift-ci-robot
added
the
lgtm
mik-dass
force-pushed
the
dev_del_improve
branch
from
4299c56
to
6662466
Compare
openshift-ci-robot
removed
the
lgtm
|
/lgtm |
openshift-ci-robot
added
the
lgtm
mik-dass
removed
the
do-not-merge/hold
|
/retest Please review the full test history for this PR and help us cut down flakes. |
7 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
What type of PR is this?
/kind bug
/kind code-refactoring
What does does this PR do / why we need it:
It improves the
odo deletecommand based on suggestions from #2763Which issue(s) this PR fixes:
Fixes: NA
How to test changes / Special notes to the reviewer:
odo delete --allshould also delete theodo-file-index.jsonfile.odo deleteprompts should also display the component name