Add Storable Helper Object #13
Conversation
Motivation: Provides helper object for client to use, avoids some foot-guns in client-side encryption. Ensures easy backward compatibility, cross-language support and efficient serialization when dealing with long-lived storage objects.
| @@ -1,6 +1,6 @@ | |||
| [package] | |||
| name = "vss-client" | |||
| version = "0.1.0" | |||
| version = "0.1.1" | |||
There was a problem hiding this comment.
Would generally prefer to have releases in separate PRs. Also, do we want to start a CHANGELOG to mention what was included in each release?
There was a problem hiding this comment.
releases in separate PRs.
This is just a patch release, there are no users, i don't think it makes any difference and is unnecessary.
I don't think CHANGELOG is required until we have an alpha or rc release.
Everything is part of vss-client core functionality. It is equivalent to overwriting 0.1.0, but sadly cargo doesn't allow that.
There was a problem hiding this comment.
This is just a patch release, there are no users, i don't think it makes any difference and is unnecessary.
I don't think CHANGELOG is required until we have an alpha or rc release. Everything is part of vss-client core functionality. It is equivalent to overwriting 0.1.0, but sadly cargo doesn't allow that.
Yeah, we can have a more formal process then.
There was a problem hiding this comment.
I don't think CHANGELOG is required until we have an alpha or rc release. Everything is part of vss-client core functionality. It is equivalent to overwriting 0.1.0, but sadly cargo doesn't allow that.
SGTM
There was a problem hiding this comment.
unrelated nit: double the in docs of GetObjectRequest (line 15).
There was a problem hiding this comment.
since this is auto-generated code, on top of that this comment is unrelated to added change, will resolve this in next PR of server side.
lets not block this one for this.
Otherwise we keep going back-and-forth on same thing and this gets auto-generated code takes 3-4 days to merge.
| /// The version of the value. Can be used by client to verify version integrity. | ||
| #[prost(int64, tag = "2")] | ||
| pub version: i64, | ||
| } | ||
| /// ErrorCodes to be used in `ErrorResponse` |
There was a problem hiding this comment.
nit: tick ErrorCode, also on lines 64, 89, 123.
There was a problem hiding this comment.
If it is defining ErrorCode, as it is here, we should write either "Error codes" or "Codes" without ticks, IMO.
There was a problem hiding this comment.
since this is auto-generated code, on top of that this comment is unrelated to added change, will resolve this in next PR of server side.
lets not block this one for this.
Otherwise we keep going back-and-forth on same thing and this gets auto-generated code takes 3-4 days to merge.
Motivation: Provides helper object for client to use,
avoids some foot-guns in client-side encryption.
Ensures easy backward compatibility, cross-language support
and efficient serialization when dealing with long-lived
storage objects.
Mainly auto-gen code from server-side PR.