-
Notifications
You must be signed in to change notification settings - Fork 102
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
support zstd formatted responses #364
Conversation
I'm unsure about this. It'd add another 150KB to the binary (pushing us over 8MB). And I don't know how much upside there is. In which cases would somebody really need it or be helped by it? (In download mode we don't even enable compression.) I'm also worried about whether this can dynamically link to the system's libzstd, since that's how (some) distros prefer to work. (The The code does look good. |
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!
@blyxxyz is there anything to change before we merge this PR?
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 like the right way to do it. I'm still a little worried about the binary size but it is nice to keep up with browsers.
Looks like right way |
Chrome versions after 123 already support zstd. (can i use zstd)
Firefox will support zstd mozilla/standards-positions#775 (comment)
Maybe we should also support zstd formatted responses.