Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Allow minor releases of lodash #1951

Merged
merged 2 commits into from
Jun 21, 2018

Conversation

cherewaty
Copy link
Contributor

Purpose

On a project where I use react-apollo and lodash, I got a warning from duplicate-package-checker that I have two versions of lodash. It'd be nice to only have one.

Approach

Allow minor releases of lodash above the specified in package.json, so projects consuming react-apollo will get the latest [email protected].

Prior work

#1478 moved lodash from devDependencies (where being locked into a specific patch release was fine) to dependencies.

@apollo-cla
Copy link

@cherewaty: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@cherewaty cherewaty force-pushed the lodash-versioning branch 2 times, most recently from 9f5225a to c4f7e0c Compare May 23, 2018 00:56
@cherewaty cherewaty force-pushed the lodash-versioning branch from c4f7e0c to 33748a9 Compare May 29, 2018 21:42
@cherewaty
Copy link
Contributor Author

#2021 upgraded lodash, but did not unpin it. Any reason to continue pinning the lodash version?

Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

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

LGTM.

I don't think that pinning this lodash was really done intentionally (but rather an artifact of previous exact-pinning by Renovate).

If it improves the situation for you by allowing some de-duplication, that seems like good enough reason to get this in.

@cherewaty cherewaty force-pushed the lodash-versioning branch from 33748a9 to d53c384 Compare May 31, 2018 13:44
@cherewaty
Copy link
Contributor Author

Thanks @abernix. I rebased to get a green light from Circle, so this should be ready to merge.

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Makes sense to me - thanks @cherewaty!

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

Successfully merging this pull request may close these issues.

4 participants