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

Return HTTP response as second return value from typed API client functions #624

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ste93cry
Copy link

Description

I'm refactoring the functions of the typed clients to return the HTTP response as second value.

Issues Resolved

Closes #619

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Sep 29, 2024

Codecov Report

Attention: Patch coverage is 99.37107% with 7 lines in your changes missing coverage. Please review.

Project coverage is 56.70%. Comparing base (06a6dc8) to head (574ee61).
Report is 70 commits behind head on main.

Files with missing lines Patch % Lines
opensearchapi/api_cat.go 95.00% 6 Missing ⚠️
opensearchapi/api_cluster.go 98.61% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #624      +/-   ##
==========================================
- Coverage   57.29%   56.70%   -0.59%     
==========================================
  Files         315      374      +59     
  Lines        9823     8036    -1787     
==========================================
- Hits         5628     4557    -1071     
+ Misses       2902     2171     -731     
- Partials     1293     1308      +15     
Flag Coverage Δ
integration 56.70% <99.37%> (+5.86%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
opensearchapi/api_aliases.go 100.00% <100.00%> (ø)
opensearchapi/api_bulk.go 100.00% <100.00%> (ø)
opensearchapi/api_cat-aliases.go 100.00% <ø> (ø)
opensearchapi/api_cat-allocation.go 100.00% <ø> (ø)
opensearchapi/api_cat-cluster_manager.go 100.00% <ø> (ø)
opensearchapi/api_cat-count.go 100.00% <ø> (ø)
opensearchapi/api_cat-fielddata.go 100.00% <ø> (ø)
opensearchapi/api_cat-health.go 100.00% <ø> (ø)
opensearchapi/api_cat-indices.go 100.00% <ø> (ø)
opensearchapi/api_cat-master.go 100.00% <ø> (ø)
... and 197 more

... and 156 files with indirect coverage changes

@dblock
Copy link
Member

dblock commented Sep 30, 2024

Big breaking change :(

If data, response, err was flipped as data, err, response, would it be easier to migrate to? Does go allow to specify the first 2 objects and omit the third?

@ste93cry
Copy link
Author

ste93cry commented Sep 30, 2024

No, it doesn’t. Also, it's conventional in Go to return error as last value. But, even though it’s a breaking change, it’s quite easy to track it and solve it as the project will not compile until the new returned value is either ignored or handled, and find and replace in most cases might be all a user needs to do if he never bothers about the response. Considering that this change is targeting the next major version, where this kind of things is expected to happen, I don’t see big downsides.

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.

[FEATURE] HTTP response as second return value of functions of the opensearchapi package
2 participants