chore: check all error returns in main code #3184
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3184 +/- ##
==========================================
- Coverage 71.24% 71.15% -0.09%
==========================================
Files 147 147
Lines 25753 25769 +16
==========================================
- Hits 18348 18337 -11
- Misses 6268 6286 +18
- Partials 1137 1146 +9
Continue to review full report at Codecov.
|
| @@ -10,11 +10,10 @@ import ( | |||
| // DeepCopy dst and src should be the same type in different API version | |||
| // dst should be pointer type | |||
| func DeepCopy(dst, src interface{}) error { | |||
| defer func() error { | |||
There was a problem hiding this comment.
Where were these deferred err responses going prior to this change? Does the caller to DeepCopy receive them?
There was a problem hiding this comment.
I think they were being ignored, because the only way to return a value from a defer in a func is a named result parameter. I'll try that change instead, because it would preserve the intent here and hopefully also placate the linter.
There was a problem hiding this comment.
I changed this and the other DeepCopy...um...copy... as specified above.
| @@ -211,7 +217,6 @@ func NewAzureClientWithClientSecretExternalTenant(env azure.Environment, subscri | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| graphSpt.Refresh() | |||
There was a problem hiding this comment.
This was returning a consistent error (once we started checking it). Need to investigate more.
There was a problem hiding this comment.
I changed this and similar calls to log.Error(err) so as not to change existing control flow.
mboersma
force-pushed
the
errcheck-the-code
branch
from
15ae299 to
64f07fa
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, mboersma 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 |
Reason for Change:
Ignoring an error returned from a func can be a dangerous thing in go, so enabling the errcheck linter is good practice. This contains only the changes necessary to main code. See #3183 for changes to test code.
Issue Fixed:
Fixes #1032.
Requirements:
Notes: