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

fix(credentials): miscellaneous typing issues #880

Merged
merged 5 commits into from
Jun 20, 2022

Conversation

TimoGlastra
Copy link
Contributor

Fixes some issues encountered in the typing. Mainly:

  • export the messages types from the core packages
  • prepend V1 and `V2 to all credential interfaces to avoid conflicts in interface names
  • use string as type for protocolVersion instead of v${string} to make it work with tsoa

@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2022

Codecov Report

Merging #880 (7560616) into main (8d67e63) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #880      +/-   ##
==========================================
+ Coverage   87.57%   87.58%   +0.01%     
==========================================
  Files         463      468       +5     
  Lines       11083    11092       +9     
  Branches     1745     1745              
==========================================
+ Hits         9706     9715       +9     
  Misses       1377     1377              
Impacted Files Coverage Δ
...kages/core/src/modules/credentials/models/index.ts 100.00% <ø> (ø)
.../modules/credentials/protocol/v2/messages/index.ts 100.00% <ø> (ø)
.../core/src/modules/credentials/CredentialsModule.ts 92.52% <100.00%> (ø)
...ages/core/src/modules/credentials/formats/index.ts 100.00% <100.00%> (ø)
...core/src/modules/credentials/formats/indy/index.ts 100.00% <100.00%> (ø)
packages/core/src/modules/credentials/index.ts 100.00% <100.00%> (ø)
...ges/core/src/modules/credentials/protocol/index.ts 100.00% <100.00%> (ø)
...dentials/protocol/revocation-notification/index.ts 100.00% <100.00%> (ø)
...les/credentials/protocol/v1/V1CredentialService.ts 95.38% <100.00%> (ø)
.../core/src/modules/credentials/protocol/v1/index.ts 100.00% <100.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d67e63...7560616. Read the comment docs.

@@ -11,7 +11,7 @@ import { V1CredentialPreview } from './V1CredentialPreview'

export const INDY_CREDENTIAL_OFFER_ATTACHMENT_ID = 'libindy-cred-offer-0'

export interface OfferCredentialMessageOptions {
export interface V1OfferCredentialMessageOptions {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether this is on purpose but we now have eg V1OfferCredentialMessageOptions and V1IssueCredentialMessageOptions. So maybe it'd be nice to have V10... or V1... consistently

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this consistent? V1<PROTOCOL>MessageOptions, or am I misreading it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Maybe I'm missing sth. Just saw that there is V10.. and V1.. - so maybe there's a reason for this?!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think you read th O in Offer as a zero. Its V1OfferCredential and not V10fferCredential :).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahahah I'm blind. thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😇🤦‍♂️

@TimoGlastra TimoGlastra merged commit ad35b08 into openwallet-foundation:main Jun 20, 2022
@TimoGlastra TimoGlastra deleted the fix/tsoa-types branch June 20, 2022 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants