USHIFT-1345: standardize logging and error messages in prerun module#1901
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@dhellmann: This pull request explicitly references no jira issue. DetailsIn response to this:
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. |
openshift-ci
bot
added
the
approved
dhellmann
force-pushed
the
prerun-logging-adjustments
branch
from
46326f5 to
32b7399
Compare
dhellmann
force-pushed
the
prerun-logging-adjustments
branch
from
32b7399 to
049e41c
Compare
pkg/admin/data/data_manager.go
Outdated
| "name", name, | ||
| ) | ||
| if err := os.RemoveAll(dm.GetBackupPath(name)); err != nil { | ||
| return errors.Wrap(err, fmt.Sprintf("Failed to delete backup %q", name)) |
There was a problem hiding this comment.
I think there's a guideline that errors should not be capitalized (and linter checks that, but here it's wrapped in fmt.Sprintf)
Edit: https://github.com/golang/go/wiki/CodeReviewComments#error-strings
There was a problem hiding this comment.
I've updated all of the error messages I could find in pkg/.
pkg/admin/data/data_manager.go
Outdated
|
|
||
| "github.com/openshift/microshift/pkg/config" | ||
| "github.com/openshift/microshift/pkg/util" | ||
| "github.com/pkg/errors" |
There was a problem hiding this comment.
This package is "3rd party" and is no longer maintained
There was a problem hiding this comment.
I replaced all uses in pkg/ in a follow-up commit.
github.com/pkg/errors is no longer maintained. Switch to using %w verb in fmt.Errorf() instead.
|
/retitle USHIFT-1345: standardize logging and error messages in prerun module |
openshift-ci
bot
changed the title
|
@dhellmann: This pull request references USHIFT-1345 which is a valid jira issue. DetailsIn response to this:
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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, pmtk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dhellmann: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
/test microshift-e2e |
4 participants
/assign @pmtk