-
-
Notifications
You must be signed in to change notification settings - Fork 959
feat: add symbol/replace
#8529
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
feat: add symbol/replace
#8529
Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
“Hey @kgryte, I went through the development guide, but I’m still unable to resolve the body issue in the test lint rule. Could you please take a look?” |
|
|
||
| // TESTS // | ||
|
|
||
| tape( 'main export is a symbol in supporting environments or null otherwise', testMain ); |
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.
@DivyanshuVortex This is not how we write tests. Please follow the style that we use throughout the project. That you deviated from that style is why CI is currently failing.
| * This symbol specifies the method that replaces matched substrings and is used | ||
| * by `String.prototype.replace()` to delegate replacement behavior to an object. | ||
| * |
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.
| * This symbol specifies the method that replaces matched substrings and is used | |
| * by `String.prototype.replace()` to delegate replacement behavior to an object. | |
| * |
| * - This well-known symbol specifies the method that replaces matched substrings. | ||
| * It is used by `String.prototype.replace()` to delegate replacement behavior. | ||
| * - The symbol is only supported in ES6/ES2015+ environments. In non-supporting | ||
| * environments, the exported value is `null`. |
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 do not line wrap descriptions.
|
|
||
| ## Usage | ||
|
|
||
| ```javascript |
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.
This is not how our READMEs are structured. Please follow that structure exactly. It is not clear why you removed the typical import statement, the typical heading, etc. The original RFC suggested simply copy-pasting and then tweaking. Here, you decided to deviate from project conventions.
kgryte
left a comment
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.
Left initial comments.
|
Ref: a68d5d9 |
|
Thank you for working on this pull request. However, we cannot accept your contribution as the issue this pull request seeks to resolve has already been addressed in a different pull request or commit. Thank you again for your interest in stdlib, and we look forward to reviewing your future contributions. |
Resolves #8479
Description
This pull request implements the RFC for introducing the package
assert/has-split-symbol-support.The implementation follows the structure and conventions of similar existing packages:
assert/has-iterator-symbol-supportassert/has-is-concat-spreadable-symbol-supportassert/has-has-instance-symbol-supportSummary of changes
Symbol.split.assert/has-iterator-symbol-support.iterator→split.Symbol.split.No additional functional differences exist beyond checking for
Symbol.split.Related Issues
symbol/replace#8479Questions
No questions for reviewers at this time.
Other
No additional notes or screenshots required.
Implementation follows stdlib package conventions.
Checklist
AI Assistance
Did you use AI assistance?
If yes, how?
Disclosure
I consulted GPT for guidance and clarity on stdlib patterns while preparing this PR.
All final implementation code and decisions were authored manually.
@stdlib-js/reviewers