-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 support for application/problem+json #2704
Conversation
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
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.
thx for the effort
lets discuss some small points
Benchmarks without variadic ctype parameter: go test -run=^$ -bench=Benchmark_Ctx_CustomJSON_Ctype -benchmem -count=4
Benchmarks with variadic ctype parameter: go test -run=^$ -bench=Benchmark_Ctx_CustomJSON_Ctype -benchmem -count=4
go test -run=^$ -bench=Benchmark_Ctx_CustomJSON_No_Ctype -benchmem -count=4
|
ok then add this feature to the JSON function |
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
@rhburt pls solve the last part |
Apologies for the delay @ReneWerner87, it should pass the linting checks now. |
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
🔥 Add support for application/problem+json #2704
Description
I have added support for RFC7807 application/problem+json alongside the existing implementation for application/json. According to the RFC, there are no mandatory fields for problem+json that distinguish it from regular json content; so, I have only added the
ProblemJson
methods that send requests/responses and associated tests that require the mime type.This is my first PR to this project so please let me know if I need to change anything. Thanks.
EDIT:
I have changed the
ProblemJSON
methods toCustomJSON
methods that take in the Content-Type header as a parameter. This allows us to support all JSON mime type extensions, such as "application/problem+json".Fixes #2701
Type of change
Checklist:
Commit formatting:
Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/