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

Merge to-device retries onto group call branch #2560

Closed
wants to merge 1 commit into from

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Aug 3, 2022

This merges #2549 onto the group-call branch

Merge is nontrivial though, so may be worth a review - I'll highlight the bits that needed actual new code.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This PR currently has none of the required changelog labels.

Add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is plus X-Breaking-Change if it's a breaking change.

@dbkr dbkr requested a review from a team as a code owner August 3, 2022 15:28
@@ -589,8 +589,7 @@ class MegolmEncryption extends EncryptionAlgorithm {
*
Copy link
Member Author

Choose a reason for hiding this comment

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

This file has new code: updated to not use the return value of encryptAndSendToDevices as it already had all the info it needed.

* @return {Promise<{contentMap, deviceInfoByDeviceId}>} Promise which
* resolves once the message has been encrypted and sent to the given
* userDeviceMap, and returns the { contentMap, deviceInfoByDeviceId }
* of the successfully sent messages.
*/
public encryptAndSendToDevices(
userDeviceInfoArr: IOlmDevice<DeviceInfo>[],
Copy link
Member Author

Choose a reason for hiding this comment

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

This function updated to use the new API and no longer returns anything.

@robintown
Copy link
Member

Oh - I might suggest waiting for #2528 to land and then merging that, since I've already resolved the conflicts with the retries PR there.

@robintown
Copy link
Member

(and otherwise, #2528 would generate yet another set of conflicts)

@dbkr
Copy link
Member Author

dbkr commented Aug 3, 2022

ah cool - ok

@dbkr
Copy link
Member Author

dbkr commented Aug 4, 2022

will re-open as necessary now that's merged

@dbkr dbkr closed this Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants