-
Notifications
You must be signed in to change notification settings - Fork 69
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: add support for response compression #723
Conversation
Hi @StarpTech If you like I can claim #674 in this repo. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
@StarpTech changes applied. Github action Router CI is falling don't know why. It is not related to my code. |
@Rutik7066 can you run |
@Aenimus Done. |
@StarpTech we can merge this now. all changes are applied. please check. |
Hi @Rutik7066, Thanks for your work on the PR! I noticed a couple of things that need to be addressed before we can merge it. First, the PR title and test files mention "request compression," but your code actually addresses "response compression." Please update the wording in the test files and PR to reflect "response compression" instead. Also, the AllowContentEncoding middleware you added checks if a request with a specific encoding can be decoded, but you didn't include support for it. Since it currently doesn't do anything, please remove this middleware. Lastly, I want to remind you that the bounty is intended to be a solo challenge, and the requirements are quite clear. The level of help you're seeking goes beyond what's expected for this challenge. Once you've made these changes, we can proceed with merging the PR. Thanks for your understanding! |
Ok I will update. |
Hi @StarpTech all changes applied. |
@StarpTech I hope everything is perfect now to merge. |
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
/claim #602
We have added middleware to compress the response.
TODO