Skip to content
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(over): Add over to compat/util #1059

Merged
merged 10 commits into from
Mar 30, 2025
Merged

feat(over): Add over to compat/util #1059

merged 10 commits into from
Mar 30, 2025

Conversation

shren207
Copy link
Contributor

@shren207 shren207 commented Mar 16, 2025

I added over function to compat. (close #1050)

The testcases follow lodash.over.

Test Results

Test List

image

Test Coverage

image

Bench Test (2x faster than lodash)

image

Copy link

vercel bot commented Mar 16, 2025

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 Mar 30, 2025 8:57am

@shren207
Copy link
Contributor Author

shren207 commented Mar 16, 2025

Pasted Graphic

Since my code didn’t pass the ci/circleci:test check, I looked into it and found that the error is coming from the test code for the retry function, not the over function I worked on.

I thought it could be a side effect of my changes, so I ran the full test suite on my local machine, but the issue didn’t reproduce.

If there’s anything I need to do to resolve this issue, please let me know.

fixed by merging main branch

@shren207 shren207 changed the title feat(over): Implement compat/over feat(over): Add over to compat Mar 23, 2025
@shren207 shren207 changed the title feat(over): Add over to compat feat(over): Add over to compat/util Mar 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.36%. Comparing base (1dd2466) to head (c479364).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1059   +/-   ##
=======================================
  Coverage   99.36%   99.36%           
=======================================
  Files         393      394    +1     
  Lines        3443     3447    +4     
  Branches     1025     1025           
=======================================
+ Hits         3421     3425    +4     
  Misses         22       22           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


describe('over', () => {
it('should create a function that invokes `iteratees`', () => {
const func = over([Math.max, Math.min]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the lodash test case, the array was not wrapped.

Not just this line, but in other lines as well, the array was not wrapped. Please check everything again. 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the thorough code review! It seems I missed some details in the finer parts.
I’ll make sure to review the upcoming PR you mentioned to better understand the parts I missed.
Once again, thanks!

Copy link
Collaborator

@dayongkr dayongkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only minor changes left in the documentation and test cases, so I will merge it first and make the adjustments later!

If you're curious about which parts needed modification, please refer to the PR that will be uploaded later.

Thank you for your contribution!!

@dayongkr dayongkr merged commit e04a913 into toss:main Mar 30, 2025
8 checks passed
@dayongkr
Copy link
Collaborator

I found out that a specific action isn't supported, so there are a lot of changes. You can check them in this PR! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add over to compat package
3 participants