-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 way to put additional headers to handshake for connecting/reconnecting, see #865 #868
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.
Would it not be better to always use a TreeMap since it is also possible for the user to provide the HTTP-Header by the constructor call?
* If the connection is already made, adding headers has no effect, | ||
* unless reconnect is called, which then a new handshake is sent.<br> | ||
* If a header with the same key already exists, it is overridden. | ||
*/ |
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.
Could you please add:
@since 1.4.1
- some description what key and value represent
here and for the other 2 new methods
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.
Thank you for the review. Of course, I am on it right now 😃
I am sorry, I couldn't understand what you meant. Could you please explain again ? |
My bad, sorry for that. My question was if we should not simple add all these entries provided with the constructor argument into the TreeMap instead of using the provided one. |
So you are saying that we should instantiate |
I think nulling is fine, we just add each header entry from the constructor (if not null) |
Hi @marci4, Could you please check out the latest commit when you have the time :). |
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. Thank you very much!
Description
Added a way to add headers in the handshake before connecting/reconnecting. Also added a method to delete a header, and to clear all headers.
Related Issue
Related to issue #865.
Motivation and Context
New Feature
How Has This Been Tested?
Types of changes
Checklist: