Skip to content

Implementation of some websocket utilities in juniper_subscriptions #617

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Implementation of some websocket utilities in juniper_subscriptions #617

wants to merge 4 commits into from

Conversation

engylemure
Copy link
Contributor

Has the objective of storing some useful implementation that can be used in the subscriptions implementation over websocket.

@engylemure engylemure mentioned this pull request Apr 10, 2020
Merged
3 tasks
LegNeato
LegNeato requested changes Apr 14, 2020
View reviewed changes
Copy link
Member

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

Thanks so much for pulling this out, it is a lot more reviewable.

LegNeato
LegNeato requested changes Apr 16, 2020
View reviewed changes
Copy link
Member

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

Shaping up nicely! Please add a test showing usage, it is hard to reason about the ergonomics for users without seeing it in use. Plus, it will be good to make sure any changes do not regress it.

@engylemure engylemure force-pushed the subscription_ws_util branch 2 times, most recently from ad3cd50 to bd7f03b Compare April 21, 2020 11:41
@engylemure
Copy link
Contributor Author

@LegNeato I moved the implementation of PR #603 subscriptions from juniper_actix here, I'm not sure if that was what you had proposed to me or if I should put this implementation somewhere else.

@engylemure engylemure force-pushed the subscription_ws_util branch 3 times, most recently from dfcad54 to 158cf70 Compare April 21, 2020 20:41
@engylemure engylemure force-pushed the subscription_ws_util branch 3 times, most recently from c5bc70e to 39620c1 Compare April 30, 2020 18:47
@engylemure engylemure force-pushed the subscription_ws_util branch from 77ac433 to df43e2f Compare June 10, 2020 21:16
@engylemure engylemure force-pushed the subscription_ws_util branch from 56013ba to 97eee2b Compare June 10, 2020 22:26
@ccbrown ccbrown mentioned this pull request Jul 26, 2020
Merged
@LegNeato
Copy link
Member

LegNeato commented Aug 2, 2020

Do the changes on latest master supercede this PR?

@engylemure engylemure closed this Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LegNeato LegNeato LegNeato requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@engylemure @LegNeato