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

Improve websocket close #1513

Conversation

maeglindeveloper
Copy link

Describe your PR and link to any relevant issues.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 66.009% when pulling 2a63394 on maeglindeveloper:improve-websocket-close into f4bf1f5 on 99designs:master.

@maeglindeveloper
Copy link
Author

hey @vektah any thoughts on this :) ?

@maeglindeveloper
Copy link
Author

Hey there, anything I can do to have it merged ? @vektah ?
Let me know

@zdraganov
Copy link
Contributor

Good job!

@maeglindeveloper
Copy link
Author

@mtibben, I saw that you merged lot of PR this month, could you please review this one ?

Copy link
Contributor

@jaredly jaredly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impl looks reasonable, but I'd like to see a test

@StevenACoffman
Copy link
Collaborator

@maeglindeveloper if you can add a test for this, I would be happy to merge it! Thanks for your patience while this project was inactive but we're catching up now and would love to have this added in!

@StevenACoffman
Copy link
Collaborator

@zdraganov what do you think of this PR now that yours has been merged?

@zdraganov
Copy link
Contributor

This can be closed. The other PR is covering this and more.

@StevenACoffman
Copy link
Collaborator

Ok, closing in favor of #1578 but anyone is welcome to open a new PR if there's something missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants