-
-
Notifications
You must be signed in to change notification settings - Fork 277
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 setter for header_table_size
#638
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.
Nice work! What do you think of adding a test to tests/h2-tests/src/client.rs
?
Used the file that most closely matched what you said. Basically a carbon copy of |
Thanks for including the test! I think this is probably good to go... Though, I'd want to ask you, since you're thinking about this topic, you might have certain expected behavior in mind. Should we be testing for that behavior? |
What I need is covered by the current test (and small fix). That is, the setting is included in the Might be worth to also test that it will throw a protocol error in the |
@seanmonstar Is there anything I can do to help this get merged? |
I don't remember what test I was asking for... What's here seems good. Just needs conflicts resolved and we can merge. |
Rebased onto |
Just making sure this is not waiting on CI? Doesn't seem like the error is related to code touched by this PR. |
Sorry, I fixed CI in #722, restarting it here. |
Signed-off-by: Sven Pfennig <s.pfennig@reply.de>
The function's description may need some refinement
Closes #637