-
-
Notifications
You must be signed in to change notification settings - Fork 907
feat: add array/base/cunone-by
#2615
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/cunone-by
#2615
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 can you please review this !!!!! |
array/base/cunone-by
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 an initial review. Main comment is that you need to carefully follow project conventions. Please try to ensure consistency with the rest of the project.
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 README needs to be updated, as it does not match the style and conventions of other similar files in this project. Please update.
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 am working on it, actually having some examinations up my sleeve.So preparing for them.
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.
The indentation of this file is off.
} | ||
} | ||
|
||
main(); No newline at end of file |
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.
You need to add trailing newlines. Please ensure EditorConfig is installed as discussed in the contributing guidelines.
if ( !isArray( y ) || y !== out ) { | ||
b.fail( 'should return the output array' ); | ||
} | ||
for ( i = 0; i < y.length; i++ ) { |
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 check is unnecessary. Remove.
@@ -0,0 +1,52 @@ | |||
{{alias}}( x, predicate ) |
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.
Match the conventions used elsewhere in this project.
"type": "opencollective", | ||
"url": "https://opencollective.com/stdlib" | ||
} | ||
} No newline at end of file |
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.
Trailing newlines here and everywhere.
}); | ||
|
||
tape( 'the function handles arrays with all elements failing the test', function test( t ) { | ||
var x = [ -1, -2, -3, -4, -5 ]; |
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.
The order of declarations violates our lint rules. Make sure you have run make init to ensure automation runs on commit and push.
} | ||
}); | ||
|
||
// Add more tests as needed No newline at end of file |
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.
Why is this comment here?
|
||
tape( 'the function returns an array', function test( t ) { | ||
var out = cunoneBy( [1, 2, 3], function() { return true; } ); | ||
t.strictEqual( Array.isArray(out), true, 'returns an array' ); |
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 don't use the builtin isArray. Use our utility package.
Additionally all build failures need to be addressed before this PR can be merged or considered for further review. |
Closing as another PR with this package has meanwhile been merged. Thanks for your efforts and interest in |
Resolves #2325 .
Description
This pull request:
Related Issues
This pull request:
@stdlib/array/base/cunone-by
#2325Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers