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

fix(fs): improve the docs and error message of ensureSymlink(Sync) #6198

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Nov 20, 2024

This PR addresses the feedback given in #6191

This PR improves the error message of fs.ensureSymlink when the link target doesn't exist. Also this improves the documentation by adding examples.

closes #6191

@github-actions github-actions bot added the fs label Nov 20, 2024
@kt3k kt3k changed the title fix(fs): improve the error message when the symlink target does not exist fix(fs): improve the docs and error message of ensureSymlink(Sync) Nov 21, 2024
@kt3k kt3k marked this pull request as ready for review November 21, 2024 03:49
Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (82ccee6) to head (9361eb0).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
fs/ensure_symlink.ts 80.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6198      +/-   ##
==========================================
- Coverage   96.58%   96.57%   -0.02%     
==========================================
  Files         532      532              
  Lines       40801    40819      +18     
  Branches     6105     6111       +6     
==========================================
+ Hits        39409    39422      +13     
- Misses       1350     1355       +5     
  Partials       42       42              

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


🚨 Try these New Features:

Copy link
Member

@satyarohith satyarohith left a comment

Choose a reason for hiding this comment

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

LGTM

Nit: update target param description that it should be relative to dirname of linkName when it's a string?

@kt3k
Copy link
Member Author

kt3k commented Nov 21, 2024

Nit: update target param description that it should be relative to dirname of linkName when it's a string?

Sounds good! Updated

@kt3k kt3k merged commit 93e0cd6 into denoland:main Nov 21, 2024
18 checks passed
@kt3k kt3k deleted the fix-fs-update-error-message-when-symlink-target-does-not-exist branch November 21, 2024 14:44
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.

fs.ensureSymlink does not work!
2 participants