-
-
Notifications
You must be signed in to change notification settings - Fork 952
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
feat(resp): add option to disable XML serialization of errors #2356
Conversation
The option is on by default
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2356 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 7712 7721 +9
Branches 1267 1269 +2
=========================================
+ Hits 7712 7721 +9 ☔ View full report in Codecov by Sentry. |
Not directly related to this PR, but it seems we also need to update the wording of docs for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just the docs need a bit polish, and I would prefer another name for the option itself (but I don't have very good suggestions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks for this improvement 👍
We do want to get rid of any XML support out of the box, although we may want to add XML handlers in the future, either out of the box, or as a recipe, or an add-on library.
I would still like to polish docstrings and the proposed newsfragment, but I can try to address that in a followup PR.
Edit: got the ball rolling in #2358.
Fixes #2355