-
Notifications
You must be signed in to change notification settings - Fork 630
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(regexp): update module example to get full jsr score #4796
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4796 +/- ##
==========================================
+ Coverage 91.88% 91.94% +0.05%
==========================================
Files 486 487 +1
Lines 41337 41385 +48
Branches 5325 5326 +1
==========================================
+ Hits 37984 38052 +68
+ Misses 3296 3276 -20
Partials 57 57 ☔ View full report in Codecov by Sentry. |
regexp/escape.ts
Outdated
* This module contains functions to escape strings for use in regular expressions. | ||
* @module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does an example make sense here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can copy over the example from the docs of only exported function but it might depend on where this data is rendered on JSR? I see that only mod.ts is rendered on the main page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it doesn't make sense to mark escape.ts as an entrypoint considering that there is only one function exported and it's also re-exported from mod.ts
I added an example anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, true. Because std/regexp
currently only has one API, setting regexp.ts
as entrypoint doesn't make much sense..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kt3k I can open a separate PR to remove it if there is no versioning issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll discuss this with Asher tomorrow. Leave it as is for now (Probably maintainers will handle this if necessary). For now I created an issue #4802
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @satyarohith!
This PR adds an example at the module level to attain full score on JSR https://jsr.io/@std/regexp/score