toss / es-toolkit

A modern JavaScript utility library that's 2-3 times faster and up to 97% smaller—a major upgrade to lodash.
https://es-toolkit.slash.page
Other
6.25k stars 263 forks source link

feat(take): Add default `count` parameter to `take` #445

Closed mass2527 closed 2 weeks ago

mass2527 commented 2 weeks ago

I think take function should also have default count parameter like takeRight

lodash take and takeRight both have default parameter.

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
es-toolkit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 29, 2024 1:25pm
codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.69%. Comparing base (b9966aa) to head (5a22896).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/toss/es-toolkit/pull/445/graphs/tree.svg?width=650&height=150&src=pr&token=8N5S3AR3C7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=toss)](https://app.codecov.io/gh/toss/es-toolkit/pull/445?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=toss) ```diff @@ Coverage Diff @@ ## main #445 +/- ## ========================================== - Coverage 99.77% 99.69% -0.08% ========================================== Files 165 165 Lines 1325 1325 Branches 357 358 +1 ========================================== - Hits 1322 1321 -1 - Misses 2 3 +1 Partials 1 1 ```
raon0211 commented 2 weeks ago

Actually, if takeRight has a default parameter, it is not what we intended. We would like our users to provide explicit arguments to our functions.

mass2527 commented 2 weeks ago

@raon0211 Thanks for reviewing.

I assumed that the es-toolkit decided to add a default parameter, similar to lodash, since takeRight already had one. If this wasn't intentional, we might consider removing the default parameter from takeRight.

raon0211 commented 2 weeks ago

Yes, that's right. We might remove the default parameter from takeRight -- that was by accident.

mass2527 commented 2 weeks ago

then i will close this PR and make another pull request that remove default parameter from takeRight