Skip to content

Use -[GRPCCall setResponseDispatchQueue] #811

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

Merged

Conversation

wilhuff
Copy link
Contributor

@wilhuff wilhuff commented Feb 17, 2018

... to dispatch GRPC callbacks directly onto the Firestore worker queue.
This saves a double-dispatch and simplifies our logic.

... to dispatch GRPC callbacks directly onto the Firestore worker queue.
This saves a double-dispatch and simplifies our logic.
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@wilhuff
Copy link
Contributor Author

wilhuff commented Feb 19, 2018

PTAL. I've merged your changes from #812 and updated comments. I started into removing writeValue: and writesFinishedWIthError: from FSTStream but ultimately decided against it because those methods know too much about how the stream behaves to make them really good candidates for separation. As it stands now FSTCallbackFilter is just a filter and that's okay.

Copy link
Contributor

@mikelehen mikelehen left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@mikelehen mikelehen merged commit dd55a4e into mikelehen/fix-stream-close-issues Feb 20, 2018
@mikelehen mikelehen deleted the wilhuff/response-dispatch-queue branch February 20, 2018 16:50
@firebase firebase locked and limited conversation to collaborators Nov 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants