Skip to content

Removed dependency on lodash.forEach #412

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

Merged
merged 7 commits into from
Feb 26, 2020
Merged

Conversation

fayyazarshad
Copy link
Contributor

Summary

to reduce package size, we are trying to gradually get rid of lodash library. This PR removes the implementation of fns.forEach and replaced it with default forEach method of arrays and Object.Keys for maps

Test plan

All tests are passing

@fayyazarshad fayyazarshad self-assigned this Feb 21, 2020
@coveralls
Copy link

coveralls commented Feb 21, 2020

Coverage Status

Coverage decreased (-0.05%) to 97.339% when pulling 2d9fa70 on fayyaz/replace_lodash_forEach into a1c18ff on master.

Copy link
Contributor

@mjc1283 mjc1283 left a comment

Choose a reason for hiding this comment

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

LGTM, but please remove filter

@fayyazarshad fayyazarshad force-pushed the fayyaz/replace_lodash_forEach branch from ec0c077 to a6fd422 Compare February 24, 2020 05:39
@fayyazarshad fayyazarshad marked this pull request as ready for review February 24, 2020 05:42
@fayyazarshad fayyazarshad requested a review from a team as a code owner February 24, 2020 05:42
@fayyazarshad fayyazarshad force-pushed the fayyaz/replace_lodash_forEach branch 2 times, most recently from dd47d8d to 37ff70c Compare February 25, 2020 05:45
@fayyazarshad fayyazarshad reopened this Feb 25, 2020
@fayyazarshad fayyazarshad self-assigned this Feb 26, 2020
@mjc1283 mjc1283 merged commit 59cdad4 into master Feb 26, 2020
@mjc1283 mjc1283 deleted the fayyaz/replace_lodash_forEach branch February 26, 2020 16:26
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.

4 participants