-
-
Notifications
You must be signed in to change notification settings - Fork 482
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 array/base/cusome-by-right
#2784
feat: add array/base/cusome-by-right
#2784
Conversation
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.
👋 Hi there! 👋
And thank you for opening your first pull request! We will review it shortly. 🏃 💨
@kgryte, could you please review the PR.
However in Should I also implement this check for |
@GittyHarsha We don't perform input validation in "base" implementations, so don't need to add such checks to this package. Thanks for asking! |
lib/node_modules/@stdlib/array/base/cusome-by-right/examples/index.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/package.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/lib/index.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/lib/assign.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/lib/assign.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/test/test.main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/test/test.main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/test/test.main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/test/test.main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/test/test.main.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.
Thanks for your contribution, @GittyHarsha! This PR should be pretty close to be able to get merged. Left a few comments with mainly code style and documentation suggestions that should be applied.
@Planeshifter, I made code changes to resolve the comments. |
lib/node_modules/@stdlib/array/base/cusome-by-right/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/array/base/cusome-by-right/docs/types/index.d.ts
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.
This PR needs a fair amount of clean-up before it should be merged.
I've only done a partial review, but this PR needs a more careful review before it can move forward. |
@kgryte and @Planeshifter,
Here, Unlike other similar packages in in the definition, I request your comment on this |
@stdlib/array/base/cusome-by-right
array/base/cusome-by-right
Resolves #2330
Description
This pull request:
Adds implementation for
@stdlib/array/base/cusome-by-right
Related Issues
This pull request:
@stdlib/array/base/cusome-by-right
#2330Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers