Open jaedle opened 4 weeks ago
This needs a changelog entry
I added a changelog entry, I hope that fits the current style. If not, I would appreciate suggestions, thanks!
Attention: Patch coverage is 88.88889%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 64.6%. Comparing base (
b6a9271
) to head (21a1bf7
).
cc @wangzlei as the soon to be owner of this instrumentation.
I reworked the error messages and added test cases / assertions.
Some edge cases might be hard to test / not worth the effort like creating http requests.
I hope you can find some time to review the changes or to comment if there is a need for changes. :pray:
Your PR now does more than just checking for non-200 HTTP status code. Other fixes should be in new PRs, not in this one.
Thanks for the feedback.
Following you gave a thumbs up on my comment above, I thought that would be appropriate.
If that’s fine for you, I would work on that within this PR.
Sorry for my confusion.
My comment was about assertion of errors in tests. You're also changing the errors content, and changing the way the response body is closed now.
My comment was about assertion of errors in tests. You're also changing the errors content, and changing the way the response body is closed now.
Rolled back in a moment, thanks.
Sorry for the change, the old way of dealing with response bodies was restored.
The committers listed above are authorized under a signed CLA.