Consistent errors for GetRef when a Ref doesn't exist #1207
Conversation
|
Fixes #1208 |
|
This is fine with me. What do you think, @gauntface ? If we do this, I believe it would fall under the category of a "minor" semver change... meaning that we would bump the middle number. It doesn't break any existing clients because it still returns an error, but the error response does change. If @gauntface is fine with this change, then LGTM. |
github/git_refs.go
Outdated
| @@ -69,6 +69,9 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref | |||
| if _, ok := err.(*json.UnmarshalTypeError); ok { | |||
| // Multiple refs, means there wasn't an exact match. | |||
| return nil, resp, errors.New("no exact match found for this ref") | |||
There was a problem hiding this comment.
Could we change the error message to "multiple matches found for this ref"?
github/git_refs.go
Outdated
| @@ -69,6 +69,9 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref | |||
| if _, ok := err.(*json.UnmarshalTypeError); ok { | |||
| // Multiple refs, means there wasn't an exact match. | |||
| return nil, resp, errors.New("no exact match found for this ref") | |||
| } else if resp.StatusCode == 404 { | |||
| // No ref, there was no match for the ref | |||
| return nil, resp, errors.New("no exact match found for this ref") | |||
There was a problem hiding this comment.
Could we change this to "no match found for this ref"?
Codecov Report
@@ Coverage Diff @@
## master #1207 +/- ##
==========================================
+ Coverage 73.39% 73.39% +<.01%
==========================================
Files 84 84
Lines 5949 5951 +2
==========================================
+ Hits 4366 4368 +2
Misses 825 825
Partials 758 758
Continue to review full report at Codecov.
|
|
@gauntface Yep that sounds sensible have fixed up this PR. |
|
Thank you, @gauntface and @pilbot ! |
No description provided.