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(path): improve API docs #4900

Merged
merged 20 commits into from
Jun 2, 2024
Merged

docs(path): improve API docs #4900

merged 20 commits into from
Jun 2, 2024

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented May 30, 2024

part of #4600 #3764

supercedes #4839

@github-actions github-actions bot added the path label May 30, 2024
@kt3k kt3k changed the title docs(paths): improve API docs docs(path): improve API docs May 30, 2024
Copy link

codecov bot commented May 30, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 92.12%. Comparing base (079e402) to head (5e1bd64).
Report is 20 commits behind head on main.

Files Patch % Lines
path/constants.ts 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4900      +/-   ##
==========================================
+ Coverage   92.02%   92.12%   +0.09%     
==========================================
  Files         488      487       -1     
  Lines       38900    38772     -128     
  Branches     5395     5388       -7     
==========================================
- Hits        35797    35717      -80     
+ Misses       3047     2999      -48     
  Partials       56       56              

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

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.

I see quite a few instances of improper casing and partial sentences. Can you please fix those?

path/basename.ts Outdated Show resolved Hide resolved
path/basename.ts Outdated Show resolved Hide resolved
kt3k and others added 3 commits May 30, 2024 17:27
Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
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.

I've fixed quite a few nits and added a few improvements. Though, I think there's a bit more work to do here:

  1. Mention the differences in behavior for OS-dependent APIs.
  2. Add ../path/posix/mod.ts and ../path/windows/mod.ts to the Super Linter, and document everything in those directories. We should get top-level documentation to a good state, then just copy the relevant details over.
  3. Improve the example code snippets (see suggestion).

path/basename.ts Show resolved Hide resolved
@denoland denoland deleted a comment May 31, 2024
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.

Big win! Great work!

@kt3k kt3k merged commit 388800f into denoland:main Jun 2, 2024
12 checks passed
@kt3k kt3k deleted the document-paths branch June 2, 2024 02:46
@iuioiua iuioiua mentioned this pull request Jun 10, 2024
6 tasks
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.

3 participants