8000 Removed dependency on lodash.map by zashraf1985 · Pull Request #409 · optimizely/javascript-sdk · GitHub
[go: up one dir, main page]

Skip to content

Removed dependency on lodash.map #409

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 4 commits into from
Feb 21, 2020
Merged

Conversation

zashraf1985
Copy link
Contributor
@zashraf1985 zashraf1985 commented Feb 20, 2020

Summary

To reduce package size, we are trying to gradually get rid of lodash library. This PR removes the map function.

Test plan

All tests pass after this change.

@coveralls
Copy link
coveralls commented Feb 20, 2020

Coverage Status

Coverage decreased (-0.0003%) to 97.559% when pulling 4f9ed50 on zeeshan/replace-lodash-map into 5a521db on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 97.564% when pulling 84b024a on zeeshan/replace-lodash-map into 9fe10a0 on master.

@zashraf1985 zashraf1985 marked this pull request as ready for review February 20, 2020 07:42
@zashraf1985 zashraf1985 requested a review from a team as a code owner February 20, 2020 07:42
@@ -33,7 +33,17 @@ module.exports = {
filter: require('lodash/filter'),
forEach: require('lodash/forEach'),
forOwn: require('lodash/forOwn'),
map: require('lodash/map'),
map: function(obj, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This map function seems unnecessary. I suggest we just update all the call sites to do what's necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was only one use. I updated that to use Object.keys and removed map method from fns altogether

@zashraf1985 zashraf1985 removed their assignment Feb 21, 2020
@mjc1283 mjc1283 merged commit 7994910 into master Feb 21, 2020
@mjc1283 mjc1283 deleted the zeeshan/replace-lodash-map branch February 21, 2020 16:36
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.

3 participants
0