Skip to content
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

fix: add comment for the readBufferSize and writeBufferSize #869

Merged

Conversation

rfyiamcool
Copy link
Contributor

summary

😅 Every time I use gorilla/websocket, I need to look at the source code and find the default size for readBufferSize and writeBufferSize.

I think the default value should be written in the comment.

@ghost
Copy link

ghost commented Nov 21, 2023

This PR updates the Upgrader documentation. The upgrader reuses the buffers returned from hijack when the buffer sizes are set to zero. It looks like these buffers are 4096 bytes, but is there any guarantee that the buffers are that size?

This PR does not update the Dialer documentation. The dialer does default to a 4096 byte buffer, so it is safe to update that documentation.

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (286c896) 70.20% compared to head (931a331) 70.20%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #869   +/-   ##
=======================================
  Coverage   70.20%   70.20%           
=======================================
  Files          11       11           
  Lines        1594     1594           
=======================================
  Hits         1119     1119           
  Misses        362      362           
  Partials      113      113           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexVulaj AlexVulaj merged commit 01b0aae into gorilla:main Dec 7, 2023
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants