-
-
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
refactor!: remove SignableRequest #463
Conversation
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.
Perfect, thanks a lot for taking this hard work!
@Xuanwo can you help release a version of reqsign? i'll raise a pr in opendal with the related api changes |
Hi, thank you very much for your effort. I plan to review the entire codebase to evaluate the overall design before making a release. This will happen today. |
sorry i did not recognize that the readme is also needed to be modified in #463 if there're other docs need to be modified together, please let me know
this PR removes SignableRequest trait and moved the convert logic into SigningContext directly.
now instead of signing an
Request
object, we choose to sign thehttp::request::Parts
.with this refactor, we can shrink the compiled binary without the duplication introduced from different generic parameters.
this is considered as a BREAKING CHANGE of the api.
fixes #460