Adding type hints for other End files delegating.py, grouping.py, ipexing.py, notifying.py, exchanging.py#355
Conversation
…uping.py, ipexing.py, notifying.py, exchanging.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
=======================================
Coverage 97.53% 97.53%
=======================================
Files 1 1
Lines 324 324
Branches 24 24
=======================================
Hits 316 316
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| sigs: string[], | ||
| recp: string[] | ||
| ): Promise<any> { | ||
| ): Promise<Operation<unknown>> { |
There was a problem hiding this comment.
@lenkan We have a follow up PR (in progress, almost there) that converts these to specific WitnessOperation, GroupOperation etc... (not generics) so should cleanup some of the things from this PR.
test-integration/credentials.test.ts
Outdated
|
|
||
| assert.strictEqual(offer.exn.p, applySaid); | ||
| assert( | ||
| 'e' in offer.exn && |
There was a problem hiding this comment.
@lenkan Hmm, I feel like this will become annoying. We could just use any instead of unknown in these scenarios, or alternatively offer a bunch of type guard functions but that list could grow quite large and hard to maintain. Thoughts?
|
@iFergal feel free to approve and merge. |
|
@kentbull Going to see if there's a nice way to serialise the exn resources from @Sotatek-Patrick-Vu Maybe we should mark as draft for a little while |
lenkan
left a comment
lenkan
left a comment
There was a problem hiding this comment.
Looks good to me.
It introduces a bit too much type casting and type checking in the tests for my taste. But I get how that happened with all the re-assigning of variables, which is a separate problem.
| /** @default null */ | ||
| groupName: string | null; | ||
| /** @default null */ | ||
| memberName: string | null; | ||
| /** @default null */ | ||
| sender: string | null; |
There was a problem hiding this comment.
Can these be omitted? Or they have to be explicitly set to null? Was thinking they are groupName?: string | null otherwise.
I haven't checked how they are used yet.
There was a problem hiding this comment.
KERIA adds them to some exns in groups().getRequest. I think we could change it to be groupName?: string; over groupName: string | null though
| let rotate_kargs = {} as RotateIdentifierArgs; | ||
| result = await delegate.identifiers().rotate('delegate1', rotate_kargs); |
There was a problem hiding this comment.
| let rotate_kargs = {} as RotateIdentifierArgs; | |
| result = await delegate.identifiers().rotate('delegate1', rotate_kargs); | |
| result = await delegate.identifiers().rotate('delegate1', {}); |
We should be able to avoid the type casting and just inline the empty object. Little less noisy.
|
@lenkan The type casting is because of Feel free to take a look ahead of time. I'd like for both of them to go in together, as I also don't like the type casting. |
4 participants
No description provided.