Skip to content

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.

5 participants