-
Notifications
You must be signed in to change notification settings - Fork 5
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
wallet_address -> account_address #121
Conversation
@@ -154,7 +154,7 @@ message RevokeInstallationRequest { | |||
|
|||
// Get all updates for an identity since the specified time | |||
message GetIdentityUpdatesRequest { | |||
repeated string wallet_addresses = 1; | |||
repeated string account_addresses = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is going to require a corresponding change in xmtp-node-go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ones in the "did not update" column feel like something we can leave. Don't know if it's worth the trouble to manage a breaking change there.
If we did care, the move would be to add a second field and fill both. Then, later, we remove the first and mark the field number as "reserved".
Yep, I agree it's not worth the trouble - if we're ever overhauling these in the future, we can bundle it in then |
Settling on consistent terminology as discussed. Depends on proto update xmtp/proto#121 Note that this will break any existing DB's - my upcoming credential update will also break existing DB's.
🎉 This PR is included in version 3.36.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Did not update:
private_preferences.proto
keystore.proto
I assume that changes to these would break existing applications