-
-
Notifications
You must be signed in to change notification settings - Fork 227
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: split request params #952
Conversation
@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel. A member of the Team first needs to authorize it. |
for more information, see https://pre-commit.ci
This reverts commit 7429479
This reverts commit 15728ed.
@sansyrox, the tests pass here. can you PTAL at the interface + an initial review, if you will. :) |
CodSpeed Performance ReportMerging #952 will not alter performanceComparing Summary
Benchmarks breakdown
|
Hey @VishnuSanal 👋 I really like the current implementation. Great job ✨ But I have a few follow ups. I will share them in a few hours |
|
robyn/robyn.pyi
Outdated
@@ -89,7 +89,7 @@ class Url: | |||
class Identity: | |||
claims: dict[str, str] | |||
|
|||
class QueryParams: | |||
class RustQueryParams: |
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.
This should be just QueryParams. No?
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.
@VishnuSanal , any update here?
Hey @VishnuSanal , some more work is needed here. Also , could you have a look at the conflicts? |
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 to me 😄 Great job ✨
why is this not merged yet? 🤔 |
Description
This PR fixes #850
Summary
This PR does split request params
PR Checklist
Please ensure that:
Pre-Commit Instructions: