-
Notifications
You must be signed in to change notification settings - Fork 42
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
Implementation of asynchronous push client #34
base: master
Are you sure you want to change the base?
Conversation
? |
Hey, sorry about the delay reviewing. Will be reviewing within a day or 2. Did you pull this locally and test this and confirm both the synchronous and async method work? There's no tests in the repo at the moment. And from the looks of it you also just cleaned up the synchronous method as well, but no functional change? In addition I think I understand what you're doing but if you can provide a slightly larger description that would be appreciated! |
#39 This might help in the testing process |
Hi, was wondering about the status of this merge request and if I could help in any way to get this merged in. |
Hi, I was looking for reasoning of this change, and review / testing to make sure it wouldn't break current applications. Is this a change you are interested in? If so, do you mind explaining what this change is doing? Will also need to resolve current merge conflicts here but if everything's good, I'm able to merge / redeploy |
Hi! |
Okay looks like this is valuable, but would need to be tested. When I have some free time, I'll download this and test it works ok with my app before i can approve this |
No description provided.