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

Allow notification apps to defer and flush the sending #21238

Merged
merged 1 commit into from
Jul 6, 2020

Conversation

nickvergessen
Copy link
Member

Signed-off-by: Joas Schilling coding@schilljs.com

@nickvergessen
Copy link
Member Author

So, let's do this for 20?

@nickvergessen nickvergessen marked this pull request as ready for review July 3, 2020 12:27
@nickvergessen nickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 3, 2020
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

I have no idea what this does :D

@nickvergessen
Copy link
Member Author

I have no idea what this does :D

It allows to interact with notifications and only sends them on flush, which means only 1 connection to the push proxy instead of of everytime. E.g. when a call in Talk is started this means 1 connection instead of 2*participants (we hacked it currently to be already 1 only, but this would be an official API for it)

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the techdebt/noid/deferrable-notification-apps branch from 902668c to 3d55915 Compare July 3, 2020 14:44
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Yes as discussed already

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code looks good 👍

@MorrisJobke MorrisJobke merged commit fb69d6d into master Jul 6, 2020
@MorrisJobke MorrisJobke deleted the techdebt/noid/deferrable-notification-apps branch July 6, 2020 11:50
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.

4 participants