-
-
Notifications
You must be signed in to change notification settings - Fork 834
feat: add support for replacing the substring before the first occurrence of a search string #843
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
Conversation
…place_before_1
lib/node_modules/@stdlib/string/base/replace-before/lib/main.js
Outdated
Show resolved
Hide resolved
@HarshitaKalani Thanks for working this! LMK if/when you'd like me to review or if you have any questions! :) |
@kgryte sure :) |
Hey @kgryte, I'm almost done with this issue but I'm not sure about the cli file in bin. Please lmk if any updates need to be made. |
@HarshitaKalani About to sign off. I'll try to review within the next day or so. |
lib/node_modules/@stdlib/string/base/replace-before/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
I have not checked logic yet, just fix indentation issues, and make sure tests pass, Welcome to stdlib @HarshitaKalani! |
I'll make all the corrections. Thanks @Pranavchiku ^_^ |
lib/node_modules/@stdlib/string/base/replace-before/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/index.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/package.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/package.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/index.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/string/base/replace-before/test/test.js
Outdated
Show resolved
Hide resolved
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. @Planeshifter Would you mind reviewing to ensure we haven't missed anything? I've gotten it to pass CI, but would be good to have another set of eyes.
@stdlib/string/base/replace-before
Looks all good to me. Thanks @HarshitaKalani for your contribution! |
This PR has now received two approvals. Will merge. Thanks again, @HarshitaKalani, for your patience and continued efforts! |
Resolves #811 .
With this PR, I've added the readme.md for replace-before issue
I'll be adding other related files after this
@stdlib-js/reviewers