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

Cleanup protobuf response #498

Closed
chacha912 opened this issue Mar 22, 2023 · 1 comment · Fixed by #614
Closed

Cleanup protobuf response #498

chacha912 opened this issue Mar 22, 2023 · 1 comment · Fixed by #614
Labels
cleanup 🧹 Paying off technical debt good first issue 🐤 Good for newcomers protocol changed 📝 Whether the protocol has changed sdk ⚒️

Comments

@chacha912
Copy link
Contributor

Description:

Remove unnecessary client information from the protobuf response.

We are currently returning client_id or client_key as follows:
https://github.com/yorkie-team/yorkie/blob/main/api/yorkie/v1/yorkie.proto#L41-L116

message ActivateClientResponse {
  string client_key = 1;
  bytes client_id = 2;
}

message DeactivateClientResponse {
  bytes client_id = 1;
}

message AttachDocumentResponse {
  bytes client_id = 1;
  string document_id = 2;
  ChangePack change_pack = 3;
}

message DetachDocumentResponse {
  string client_key = 1;
  ChangePack change_pack = 2;
}

message RemoveDocumentResponse {
  string client_key = 1;
  ChangePack change_pack = 2;
}

message PushPullChangesResponse {
  bytes client_id = 1;
  ChangePack change_pack = 2;
}

We only need to return the client_id when activating, and the rest seems unnecessary.
It would be better to clean up.

Why:
to make protobuf concise

@hackerwins hackerwins added good first issue 🐤 Good for newcomers cleanup 🧹 Paying off technical debt protocol changed 📝 Whether the protocol has changed labels Mar 22, 2023
This was referenced Aug 18, 2023
@hackerwins
Copy link
Member

hackerwins commented Aug 18, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt good first issue 🐤 Good for newcomers protocol changed 📝 Whether the protocol has changed sdk ⚒️
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants