Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Add ability to set custom WebSocket protocols in client #477

Merged
merged 3 commits into from
Mar 11, 2019
Merged

Add ability to set custom WebSocket protocols in client #477

merged 3 commits into from
Mar 11, 2019

Conversation

pkosiec
Copy link
Contributor

@pkosiec pkosiec commented Oct 18, 2018 •
edited
Loading

Hello,
I'm using the library on my front-end application and I would like to have the ability to set custom WebSocket protocols for client.

I've seen an existing issue with this feature request - that's why I'm opening this PR. Hope you'll like it!

TODO:

  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change

Resolves #404

@apollo-cla
Copy link

@pkosiec: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@ghost ghost added the feature New addition or enhancement to existing solutions label Oct 18, 2018
@pkosiec pkosiec changed the title Add ability to set custom WebSocket protocols Add ability to set custom WebSocket protocols in client Oct 18, 2018
@pkosiec
Copy link
Contributor Author

pkosiec commented Jan 3, 2019 •
edited
Loading

Hello @mistic, @helfer! Could you please take a look at this? Thanks!

@pkosiec
Copy link
Contributor Author

pkosiec commented Mar 8, 2019

@abernix @trevorblades @hwillson @trevor-scheer Could you please review this PR? Thank you 🙂

mistic
mistic approved these changes Mar 8, 2019
View reviewed changes
Copy link
Contributor

@mistic mistic left a comment

Choose a reason for hiding this comment

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

That looks good to me @pkosiec

@pkosiec
Copy link
Contributor Author

pkosiec commented Mar 10, 2019

@mistic Great! Would you be able to merge it?

@mistic
Copy link
Contributor

mistic commented Mar 11, 2019

@pkosiec let's just wait for a review from @trevorblades

@pkosiec
Copy link
Contributor Author

pkosiec commented Mar 11, 2019

Sure. Fingers crossed!

@trevorblades trevorblades self-requested a review March 11, 2019 17:40
trevorblades
trevorblades approved these changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@trevorblades trevorblades left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @pkosiec 😄

hwillson
hwillson approved these changes Mar 11, 2019
View reviewed changes
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Great stuff - thanks @pkosiec!

@trevorblades trevorblades merged commit cf3f492 into apollographql:master Mar 11, 2019
@pkosiec
Copy link
Contributor Author

pkosiec commented Mar 11, 2019

Awesome, thanks for merging!

@snyk-bot snyk-bot mentioned this pull request Apr 24, 2020
Closed
@ebarault ebarault mentioned this pull request May 15, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hwillson hwillson hwillson approved these changes

@trevorblades trevorblades trevorblades approved these changes

@mistic mistic mistic approved these changes

Assignees
No one assigned
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pkosiec @apollo-cla @mistic @hwillson @trevorblades