add warning log entry for removing non-exist package #779
Conversation
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @akihikokuroda. Thanks for your PR. I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
akihikokuroda
force-pushed
the
index-rm
branch
from
d51da2e to
4004308
Compare
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
pkg/sqlite/load.go
Outdated
| return err | ||
| } | ||
| if len(csvNames) == 0 { | ||
| logrus.WithFields(logrus.Fields{"pkg": packageName}).Warnf("package doesn't exist") |
There was a problem hiding this comment.
Should this be an error instead of a warning?
There was a problem hiding this comment.
We should probably just return here.
There was a problem hiding this comment.
Do we need some testing if we're changing behavior here?
There was a problem hiding this comment.
Thanks for reviewing. I'll change the warning to the error and return here. I'll look into the test change, too.
There was a problem hiding this comment.
I think the linter error is just fixed by running go-fmt
Codecov Report
@@ Coverage Diff @@
## master #779 +/- ##
==========================================
+ Coverage 50.90% 50.94% +0.03%
==========================================
Files 102 102
Lines 8753 8755 +2
==========================================
+ Hits 4456 4460 +4
+ Misses 3458 3456 -2
Partials 839 839
Continue to review full report at Codecov.
|
akihikokuroda
force-pushed
the
index-rm
branch
from
4004308 to
a8eba18
Compare
|
I changed to return the error to caller so that the caller can handle the error. I also updated the test. Thanks! |
Signed-off-by: akihikokuroda <akuroda@us.ibm.com>
akihikokuroda
force-pushed
the
index-rm
branch
from
a8eba18 to
7b2d1a9
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akihikokuroda, joelanford 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
bot
added
the
approved
6 participants
Description of the change:
Adding a warning log entry when
opm index rmis issued for a non-existing packageMotivation for the change:
Closes #675
Reviewer Checklist
/docs