-
Notifications
You must be signed in to change notification settings - Fork 251
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
Reconnect jitter and delay #99
base: master
Are you sure you want to change the base?
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.
LGTM.
lgtm |
@kozlovic this PR has been open for a while - is this ok to merge now or do we need other changes? |
@gcolliso Oh wow.. yes this is an old one. We could add clients that have this feature before the merge, or do it after. I could add the C client to that PR before making it public. |
@kozlovic you can add before the merge. If you know other clients and who may be able to contribute the code, feel free to tag them in this PR. |
Added for Go client for now, need to add when others are avail.
Document that libraries will use default values if the ReconnectJitter option is not specified. Signed-off-by: Ivan Kozlovic <[email protected]>
Signed-off-by: Ivan Kozlovic <[email protected]>
ddb252b
to
600cbb6
Compare
@gcolliso Ok, I have added the C client. So now there is Go and C. Pinging @aricart, @wallyqs, @scottf, @ColinSullivan1 to see if their library has this feature, and if so, if possible to add to it (could be done after merge of this PR or added as new commits). |
@kozlovic yes it is implemented in JavaScript |
@aricart So will you update this PR with code example or do you want to do it after this PR is merged? |
@kozlovic Ginger is currently updating the doc for JavaScript, so when that is done, I can update it - the tab content in the current PR is documenting the old clients. |
Added for Go client for now, need to add when others are avail.