-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Update minimal websocket client example. #9455
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
Update minimal websocket client example. #9455
Conversation
2f6f2c2 to
ec0dcc3
Compare
75d79fb to
653dbea
Compare
|
Thanks for the corrections. This is my first PR other than a few small fixes, so I wasn't aware of all of the guidelines. I fixed everything you said. |
skyace65
left a comment
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.
Code works for me
tutorials/networking/websocket.rst
Outdated
| # The code will be -1 if the disconnection was not properly notified by the remote peer. | ||
| var code = socket.get_close_code() | ||
| print("WebSocket closed with code: %d. Clean: %s" % [code, code != -1]) | ||
| set_process(false) # Stop processing. |
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.
Just one final nitpick, the code should be indented with spaces, not tabs, otherwise this is all 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.
I converted the tabs to spaces. If it matters, shouldn't this be part of the lint though?
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.
I'm not sure why it isn't but I think it's a limitation of the system, unsure
653dbea to
6324f35
Compare
6324f35 to
2afeaaf
Compare
|
Thanks! |
|
Not sure why the PR changed the websocket address, but we moved away from
the one used in this PR because it was very unreliable (i.e. was offline
for a very long time).
…On Wed, Jun 5, 2024, 20:53 AThousandShips ***@***.***> wrote:
Thanks!
—
Reply to this email directly, view it on GitHub
<#9455 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM4C3U7U3NL5UPCFEFDRFTZF5NCJAVCNFSM6AAAAABIXLQZJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJQG42DCNJVGE>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
I changed the url because the previous one wasn't working, and all of the minimal examples I saw used the one I'm using here. |
The new example is a combination of the old example and the example from the class reference.
This fixes #9138 and fixes #6631.