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

Improve #3215 implementation #3226

Conversation

florianduros
Copy link
Contributor

@florianduros florianduros commented Mar 23, 2023

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)

Following comments on #3215

Sliding sync change will be done in another PR.


This change is marked as an internal change (Task), so will not be included in the changelog.

@florianduros florianduros added the T-Task Tasks for the team like planning label Mar 23, 2023
@florianduros florianduros force-pushed the florianduros/tech/improve-one-time-key-count-unused-fallback-keys branch from 8fbb149 to 8e5be5b Compare March 23, 2023 17:13
@texuf
Copy link
Contributor

texuf commented Mar 23, 2023

I know it's still a draft, but could a summary of how you improved the key upload request in your description?

@florianduros florianduros changed the title Improve key upload request Improve #3215 implementation Mar 24, 2023
@florianduros
Copy link
Contributor Author

@texuf The behaviour is unchanged, it's mainly refactoring, comments and testing. Following @richvdh suggestions

@florianduros florianduros marked this pull request as ready for review April 3, 2023 16:51
@florianduros florianduros requested a review from a team as a code owner April 3, 2023 16:51
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thank you! a few more things though.

src/sync.ts Outdated Show resolved Hide resolved
src/common-crypto/CryptoBackend.ts Outdated Show resolved Hide resolved
src/crypto/index.ts Outdated Show resolved Hide resolved
src/rust-crypto/rust-crypto.ts Outdated Show resolved Hide resolved
src/crypto/index.ts Outdated Show resolved Hide resolved
spec/integ/crypto.spec.ts Outdated Show resolved Hide resolved
spec/integ/crypto.spec.ts Outdated Show resolved Hide resolved
spec/unit/rust-crypto/rust-crypto.spec.ts Show resolved Hide resolved
src/common-crypto/CryptoBackend.ts Outdated Show resolved Hide resolved
src/common-crypto/CryptoBackend.ts Outdated Show resolved Hide resolved
src/crypto/index.ts Outdated Show resolved Hide resolved
src/rust-crypto/rust-crypto.ts Outdated Show resolved Hide resolved
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thanks!

this.updateOneTimeKeyCount(oneTimeKeysCounts["signed_curve25519"] || 0);
}

if (unusedFallbackKeys) {
if (unusedFallbackKeys !== undefined) {
// If `unusedFallbackKeys` is truthy, that means `device_unused_fallback_key_types`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// If `unusedFallbackKeys` is truthy, that means `device_unused_fallback_key_types`
// If `unusedFallbackKeys` is defined, that means `device_unused_fallback_key_types`

…me-key-count-unused-fallback-keys' into florianduros/tech/improve-one-time-key-count-unused-fallback-keys
@florianduros florianduros added this pull request to the merge queue Apr 5, 2023
Merged via the queue into develop with commit 2daa429 Apr 5, 2023
@florianduros florianduros deleted the florianduros/tech/improve-one-time-key-count-unused-fallback-keys branch April 5, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants