digitalbazaar / edv-client

An Encrypted Data Vault Client

BSD 3-Clause "New" or "Revised" License

13 stars 9 forks source link

Add jsdocs and fix documentation. #73

Closed JSAssassin closed 3 years ago

JSAssassin commented 4 years ago

Note: jsdoc2md *.js > api.md doesn't seem to be generating documents for all the .js files. I am not sure if I am using it incorrectly or maybe it just doesn't work.

Addresses #72

mattcollier commented 4 years ago

@JSAssassin it seems as though you may have been unaware of this outstanding PR from @mandyvenables https://github.com/digitalbazaar/edv-client/pull/71

It appears that she update the jsdocs in large swaths of code as well. This will of course produce merge conflicts.

@JSAssassin whenever you break ground on a new PR, it's always a good idea to familiarize yourself with any open PRs on the project. This helps to avoid duplication of effort.

JSAssassin commented 4 years ago

@mattcollier yeah I didn't check the open PRs, sorry about that. I'll keep that in mind and will do next time.

codecov-io commented 3 years ago

Codecov Report

Merging #73 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   75.56%   75.56%           
=======================================
  Files           5        5           
  Lines         577      577           
=======================================
  Hits          436      436           
  Misses        141      141

Impacted Files	Coverage Δ
EdvClient.js	`74.81% <ø> (ø)`
EdvDocument.js	`100.00% <ø> (ø)`
IndexHelper.js	`72.29% <ø> (ø)`

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 4a70c37...1887678. Read the comment docs.

aljones15 commented 3 years ago

sorry about this one @JSAssassin I guess I should have checked that there is an outstanding PR for jsdocs updates in edv client before noticing it.

EDIT: see comment below it looks like the jsdoc errors are still there.

aljones15 commented 3 years ago

@mattcollier my apologies about this I noticed some issues with the jsdoc in edv client in another PR in web-profile-manager.

Those issues are still there though:

https://github.com/digitalbazaar/edv-client/blob/4a70c37890a54885f52e5f28cd11b8a4dafab9c1/EdvClient.js#L437-L458

options.doc has a type {string} for instance, but then there is a check to ensure it an object.

the same issue occurs here:

https://github.com/digitalbazaar/edv-client/blob/4a70c37890a54885f52e5f28cd11b8a4dafab9c1/EdvClient.js#L344-L376

in both cases options.doc is documented as a string when it should be an object. I might add using a string in both cases will result in an error.

@JSAssassin @dmitrizagidulin

JSAssassin commented 3 years ago

@aljones15 I have fixed the options.doc type in jsdocs. Do we need auto api document generation here? I tried to set it up, but jsdoc2md *.js > api.md doesn't seem to be working.

aljones15 commented 3 years ago

@aljones15 I have fixed the options.doc type in jsdocs. Do we need auto api document generation here? I tried to set it up, but jsdoc2md *.js > api.md doesn't seem to be working.

that has been broken for awhile. I would not worry about it unless specifically assigned to it. Thanks for the doc fix.