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.

Sign up for GitHub

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

Jump to bottom

feat: Update CC to 55 and adopt E2EI changes [WPB-7126] #2691

Merged
merged 4 commits into from
Apr 3, 2024
Merged

feat: Update CC to 55 and adopt E2EI changes [WPB-7126] #2691

merged 4 commits into from
Apr 3, 2024

Conversation

borichellow
Copy link
Contributor

What's new in this PR?

  1. update CC version to 55
  2. update mpping for WireIdentity
  3. update calculation of users E2EI verification status: if any WireIdentity has no certificate (no metter if its status is VALID) -> user is not verified.

@borichellow borichellow self-assigned this Apr 2, 2024
mchenani
mchenani approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@mchenani mchenani left a comment

Choose a reason for hiding this comment

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

LGTM, with a question.

cryptography/src/commonJvmAndroid/kotlin/com.wire.kalium.cryptography/MLSClientImpl.kt Outdated
value.displayName,
value.domain,
value.certificate,
value.x509Identity?.handle ?: "",
Copy link
Member

Choose a reason for hiding this comment

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

Question: since the inner object is nullable. Imo we should return null instead of hardcoded values?

MohamadJaara
MohamadJaara previously approved these changes Apr 2, 2024
View reviewed changes
cryptography/src/commonJvmAndroid/kotlin/com.wire.kalium.cryptography/MLSClientImpl.kt Outdated
Comment on lines 329 to 332
value.x509Identity?.handle ?: "",
value.x509Identity?.displayName ?: "",
value.x509Identity?.domain ?: "",
value.x509Identity?.certificate ?: "",
Copy link
Member

Choose a reason for hiding this comment

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

please do not add default empty strings, if the value is supposed to be nullable now then keep is nullable

@MohamadJaara MohamadJaara self-requested a review April 2, 2024 13:45
MohamadJaara
MohamadJaara requested changes Apr 2, 2024
View reviewed changes
Copy link
Member

@MohamadJaara MohamadJaara left a comment

Choose a reason for hiding this comment

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

change to keep nullable values as it is

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 2, 2024 •
edited
Loading

Test Results

2 850 tests   2 721 ✔️  25s ⏱️
   487 suites     129 💤
   487 files           0 ❌

Results for commit 9e90b2c.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Apr 2, 2024 •
edited
Loading

Datadog Report

All test runs 147bb9d 🔗

✅ 2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Wall Time Test Service View
kalium-ios 0 0 0 2721 129 7m 33.1s Link
kalium-jvm 0 0 0 2851 123 9m 5.04s Link

@MohamadJaara MohamadJaara merged commit 59cb1e5 into release/candidate Apr 3, 2024
17 checks passed
@MohamadJaara MohamadJaara deleted the feat/update_cc_to_55_and_adopt_e2ei_changes branch April 3, 2024 15:53
github-actions bot pushed a commit that referenced this pull request Apr 3, 2024
@borichellow @github-actions
feat: Update CC to 55 and adopt E2EI changes [WPB-7126] (#2691)
10621bf 
* feat: Update CC to 55 and adotp E2EI changes

* Fix according to the review

* CodeStyle fix

* Fix tests
@github-actions github-actions bot mentioned this pull request Apr 3, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Apr 26, 2024
@github-actions @borichellow
feat: Update CC to 55 and adopt E2EI changes [WPB-7126] (#2691) (#2693)
64127a5 
* feat: Update CC to 55 and adotp E2EI changes

* Fix according to the review

* CodeStyle fix

* Fix tests

Co-authored-by: boris <boris.safonov@appunite.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MohamadJaara MohamadJaara MohamadJaara approved these changes

@mchenani mchenani mchenani approved these changes

@vitorhugods vitorhugods Awaiting requested review from vitorhugods

Assignees

@borichellow borichellow

Labels
🚨 Potential breaking changes 👕 size: M type: feature ✨
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@borichellow @MohamadJaara @mchenani