Page MenuHomePhabricator

[Identity] Use content to describe main olm account
ClosedPublic

Authored by jon on Apr 27 2023, 8:22 AM.
Tags
None
Referenced Files
F3391231: D7671.diff
Sat, Nov 30, 2:52 AM
Unknown Object (File)
Mon, Nov 25, 8:52 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Unknown Object (File)
Mon, Nov 25, 8:51 PM
Subscribers

Details

Summary

To avoid overloaded terms, use content instead of identity when
describing the main olm account.

Part of https://linear.app/comm/issue/ENG-3816

Test Plan

Gates pass

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 27 2023, 8:25 AM
Harbormaster failed remote builds in B18879: Diff 25857!

Apply changes to keyserver as well

ashoat requested changes to this revision.Apr 27 2023, 2:09 PM

Only looked at .proto changes

shared/protos/identity_client.proto
78 ↗(On Diff #25861)

I don't think this should've been renamed. Actually, the fact that you accidentally renamed this is a strong signal that our renaming is a good idea

See your comment below:

Sessions for users will contain both ContentKeys and NotifKeys

IdentityKeyInfo is a good name to cover both of these. It was confusing before, when "Identity" was overloaded to mean two things. But now that it only means one thing, I think it's a good name to keep

This revision now requires changes to proceed.Apr 27 2023, 2:09 PM
ashoat requested changes to this revision.Apr 28 2023, 11:18 AM

Please make sure you're going through your changes carefully, and not just submitting a find-replace diff. The term "identity" is used in many places, and not all of those places correspond to the notif vs. content dichotomy

shared/protos/identity_client.proto
86 ↗(On Diff #25906)

What does it mean to correlate a wallet with the "content" of a device?

This revision now requires changes to proceed.Apr 28 2023, 11:18 AM
jon marked an inline comment as done.

Clarify content vs identity in some instances

shared/protos/identity_client.proto
86 ↗(On Diff #25906)

rephrased this, as it was ambiguous what "identity of a device" meant originally

Accepting for the .proto changes. Can't exactly speak for completeness on the Rust side of things, but the changes that have been made seem okay

This revision is now accepted and ready to land.Apr 29 2023, 8:08 AM