search

decentralized-identity / veramo

A JavaScript Framework for Verifiable Data
https://veramo.io
Apache License 2.0
414 stars 130 forks source link

[proposal] Update doc comments to prevent misunderstanding #1356

Closed hylim-tech-lover closed 3 months ago

hylim-tech-lover commented 4 months ago

Is your feature request related to a problem? Please describe.

I noticed that the following code doesn't describe the field correctly (Line 114 and 124): https://github.com/decentralized-identity/veramo/blob/4696b5b2c9c9a6c0c8d2fb29e9789c2d63bd197c/packages/core-types/src/types/ICredentialIssuer.ts#L101-L126

Describe the solution you'd like

  /**
-  * If this parameter is true, the resulting VerifiablePresentation is sent to the
+  * If this parameter is true, the resulting VerifiableCredential is sent to the
   * {@link @veramo/core-types#IDataStore | storage plugin} to be saved.
   *
   * @deprecated Please call
   *   {@link @veramo/core-types#IDataStore.dataStoreSaveVerifiableCredential | dataStoreSaveVerifiableCredential()} to
   *   save the credential after creating it.
   */
  save?: boolean

  /**
-   * The desired format for the VerifiablePresentation to be created.
+   * The desired format for the VerifiableCredential to be created.
   */
  proofFormat: ProofFormat

Additional context Correct if I am wrong as I believe the above is for VerifiableCredential and not VerifiablePresentation.

As a sidenote, I also noticed the lines below is point to nothing. Could you verify is it inserted by accidental or to be addressed in future?

https://github.com/decentralized-identity/veramo/blob/4696b5b2c9c9a6c0c8d2fb29e9789c2d63bd197c/packages/core-types/src/types/ICredentialIssuer.ts#L155-L160

mirceanis commented 4 months ago

You are correct. The comments should refer to Verifiable Credential.

The floating comment refers to the verification result. Now that data type is collectively called IVerifyResult and it's inline doc should be updated to refer to credentials as well as presentations.

There is a similar floating comment in the ICredentialVerifier.ts file.

Feel free to open a PR with the fix.

hylim-tech-lover commented 4 months ago

The floating comment refers to the verification result. Now that data type is collectively called IVerifyResult and it's inline doc should be updated to refer to credentials as well as presentations.

Alright noted. I will be ignoring those.

Feel free to open a PR with the fix.

Sure thing. I will be making the PR shortly after today.