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

docs(toml): lint @std/toml docs #4799

Merged
merged 11 commits into from
May 30, 2024
Merged

docs(toml): lint @std/toml docs #4799

merged 11 commits into from
May 30, 2024

Conversation

littledivy
Copy link
Member

No description provided.

@littledivy littledivy requested a review from kt3k as a code owner May 21, 2024 12:17
Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.36%. Comparing base (7717c29) to head (ffb98f9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4799   +/-   ##
=======================================
  Coverage   92.36%   92.36%           
=======================================
  Files         488      488           
  Lines       41603    41603           
  Branches     5409     5409           
=======================================
  Hits        38425    38425           
  Misses       3122     3122           
  Partials       56       56           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lucacasonato
Copy link
Member

The docs for this module are still very lacking. Neither parse nor stringify has an example, and the docs are quite poor in general.

@littledivy
Copy link
Member Author

littledivy commented May 21, 2024

Oh, the lint should probably detect that? I got an error for missing @example in crypto

@lucacasonato
Copy link
Member

Yes, I agree it should. See comment at the top of _tools/check_docs.ts: // TODO(iuioiua): Add support for classes and methods. :)

@github-actions github-actions bot added the toml label May 22, 2024
Copy link
Collaborator

@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.

Can you please add toml/mod.ts to the file list in the lint:docs task? Ignore - I see we've moved this all into the doc checker now.

toml/stringify.ts Outdated Show resolved Hide resolved
toml/stringify.ts Outdated Show resolved Hide resolved
* @param srcObj
*
* @example Stringify an object
* ```ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please use an assertion from std/assert here? It ensures the example is correct. Ditto for other examples.

Copy link
Member

Choose a reason for hiding this comment

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

Added the assertions

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k dismissed iuioiua’s stale review May 30, 2024 04:07

feedbacks are addressed

@kt3k kt3k merged commit 079e402 into denoland:main May 30, 2024
12 checks passed
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.

None yet

4 participants