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

added JsDoc example to fetchValues function #497

Closed
wants to merge 7 commits into from

Conversation

Nkiriobasi
Copy link

@Nkiriobasi Nkiriobasi commented Sep 4, 2023

added documentation using JsDoc

Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @Nkiriobasi! JSDocs and comments aim to add missing context from the implementation. So, a few notes:

  • Adding line-by-line comments doesn't add context, as the reader can just read the code with the same level of understanding.
  • As stated in the issue, each function should have an @example snippet.
  • A @param directive isn't required if an argument is simple enough.

With these specific functions, for example, you probably just want a simple description as the first line, then an @example snippet. Check this out as a good example.

Also, please be sure deno task ok passes before committing.

Please let me know if you need anything.

@Nkiriobasi
Copy link
Author

Alright, no problem. I'll refactor.

@Nkiriobasi
Copy link
Author

Please I'm using one PR with different commit indicating different files. it was not intentional.

@iuioiua
Copy link
Contributor

iuioiua commented Sep 5, 2023

Please run deno fmt

iuioiua added a commit that referenced this pull request Sep 7, 2023
@iuioiua
Copy link
Contributor

iuioiua commented Sep 7, 2023

Hi @Nkiriobasi, I'll close this PR without merging as I created #533, which superseded this one. I copied some bits from this PR to that one. Please feel free to take a look to get a feel for the style I'm hoping for. Either way, thank you.

@iuioiua iuioiua closed this Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants