chore remove a nil deference #765
Conversation
Signed-off-by: emirot <[email protected]>
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
- Coverage 56.79% 56.53% -0.26%
==========================================
Files 37 37
Lines 4004 4008 +4
==========================================
- Hits 2274 2266 -8
- Misses 1446 1455 +9
- Partials 284 287 +3 ☔ View full report in Codecov by Sentry. |
internal/scms/gitlab/pullrequest.go
Outdated
| metrics.RecordSCMCall(repo, metrics.SCMAPIPullRequest, metrics.SCMOperationList, resp.StatusCode, time.Since(start), nil) | ||
| } | ||
| if err != nil { | ||
| return false, "", time.Time{}, fmt.Errorf("failed to list pull requests: %w", err) |
There was a problem hiding this comment.
Does the gitlab client return an error for non-200 responses? If so, we still want to make sure we push the metric for the failed request.
There was a problem hiding this comment.
From what I see it does return an error for non-200 response code https://gitlab.com/gitlab-org/api/client-go/-/blob/main/merge_requests.go#L316
There was a problem hiding this comment.
Then I don't think we can move the metrics call down...
But we can still add the if resp == nil check, I think that's valid and fine.
There was a problem hiding this comment.
Thanks @crenshaw-dev for the explanation, I have updated
Signed-off-by: emirot <[email protected]>
3 participants
What
Return error if resp is nil
Why
Nilway detected