Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit error return #2469

Merged
merged 4 commits into from Sep 17, 2022
Merged

Add explicit error return #2469

merged 4 commits into from Sep 17, 2022

Conversation

mo1ein
Copy link
Contributor

@mo1ein mo1ein commented Sep 17, 2022

No description provided.

@codecov
Copy link

codecov bot commented Sep 17, 2022

Codecov Report

Merging #2469 (132029f) into master (d611561) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2469   +/-   ##
=======================================
  Coverage   98.03%   98.03%           
=======================================
  Files         122      122           
  Lines       10692    10695    +3     
=======================================
+ Hits        10482    10485    +3     
  Misses        144      144           
  Partials       66       66           
Impacted Files Coverage Δ
github/repos_contents.go 89.54% <100.00%> (+0.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines 322 to 323
}
return parsedURL, newResponse(resp), nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is another thing that is inconsistent across this repo, but while I'm thinking about it, let's go ahead and add a blank line between the error handling and the final return, please.

Suggested change
}
return parsedURL, newResponse(resp), nil
}
return parsedURL, newResponse(resp), nil

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Done.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 17, 2022

Note that I'm going to change the title of this PR because it is technically not an "unhandled error"... it is simply an error that is not explicitly tested for.

@gmlewis gmlewis changed the title Fix unhandled error Add explicit error return Sep 17, 2022
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @mo1ein !
LGTM.
Merging.

@gmlewis gmlewis merged commit 30d1431 into google:master Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants