we need to get the keyserver's keys to set up an Olm session on the client.
Depends on D9345
Details
Details
successfully retrieved my local keyserver's keys by calling the CommRustModule method from js
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I have some questions, if those don't make sense I'll try to unblock this as soon as possible
| lib/types/identity-service-types.js | ||
|---|---|---|
| 11 | based on Rust types it's more like +socialProof: ?string but don't have a strong opinion on that | |
| 16–17 | Why this is not defined in the Rust struct? If this is different than struct OutboundKeyInfoResponse shouldn't be named differently to avoid confusion? | |
| native/native_rust_library/src/lib.rs | ||
| 652 | After looking on master this method is still not implemented, and don't see this in the stack - so not really sure how it was tested. Am I missing something? | |
| 659 | this is really confusing, isn't get() a better option? | |
| native/schema/CommRustModuleSchema.js | ||
| 54 | Based on Rust code looks like identifierType can have only two values, walletAddress and username, is that correct? If yes - probably it will be safe to force those two types in Flow, so it should be identifierType: 'walletAddress' | 'username',. I don't know but guessing codegen can't handle type unions - if that's true you can use a trick we use in different schemas with type for codegen and type exported for Flow, see UtilsModuleSpec. | |
Comment Actions
address feedback
| lib/types/identity-service-types.js | ||
|---|---|---|
| 11 | makes sense | |
| 16–17 | fixed, thanks | |
| native/native_rust_library/src/lib.rs | ||
| 652 | sorry, meant to put that diff up first. it's up now as you've already seen :) | |
| 659 | we actually use this pattern a lot because get() only returns a reference. i'd prefer that we continue this way | |
| native/schema/CommRustModuleSchema.js | ||
| 54 | ||
Comment Actions
Looks good to me, thanks for addressing all comments
| native/native_rust_library/src/lib.rs | ||
|---|---|---|
| 659 | ahh right, looks good then | |