-
Notifications
You must be signed in to change notification settings - Fork 36
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 a request decorator #422
base: main
Are you sure you want to change the base?
Conversation
looks fine - curious if you considered doing this as an implicit to keep out ofarg list (but fine as consistent with other parameters there) |
I briefly thought about and was not convinced that this kind of decorator should be implicit. I do not have a strong opinion about it all so if we think it is better as an implicit, I will happily add it to the implicit args. |
i've no strong opinion - gregor? |
LGTM. Is there a risk of breaking binary compatibility? Anyhow, would be nice to make the same change in the http4s generators. |
http4s and ning are part of the PR. This does break binary compatibility as the signatures are changed. There is no way around it as an "older" version of the class could always be loaded that does not contain the updated feature. @gheine How do we go about it? |
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
Goal is to allow users to modify the request being issued.
Typical use case includes: