-
Notifications
You must be signed in to change notification settings - Fork 8
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
remove lodash #66
remove lodash #66
Conversation
Hey @lukeed 👋 Thanks for your contribution! We'll review your change and get back to you shortly |
Hey @lukeed I'm happy with the changes you've made but we require all commits to be signed before merging, it might be possible to sign the older commit with something like
|
04b1970
to
4d84abe
Compare
Sorry for the delay. I ran the command, which doesn't seem to have done anything which is odd considering my others elsewhere are signed. I'm totally fine/happy if you'd rather cherry-pick the diffs. It's not an involved PR and I'd rather see this not get caught up in process-like things, especially when its my own doing 🙃 |
lodash is huge & you were using a tiny portion of its insignificant bits Signed-off-by: Luke Edwards <[email protected]>
Signed-off-by: Luke Edwards <[email protected]>
Signed-off-by: Luke Edwards <[email protected]>
Fixed. I was on a machine that didnt have a GPG configured 🤦 |
Great, thanks for your contribution here @lukeed this should give us a huge saving on bundle size |
Hi there 👋
lodash
is a huge dependency & you were using a tiny portion of it. Really you just neededsnakeCase
-- the rest were native stand-ins w/ alodash.
prefix.Ran tests locally & all were passing