greenkeeperio / greenkeeper-lockfile

:lock: Your lockfile, up to date, all the time
https://greenkeeper.io
183 stars 73 forks source link

fix(yarn): preserve caret ranges #69

Closed jeysal closed 6 years ago

jeysal commented 6 years ago

Currently, the lockfile commit for a dependency that is declared with a caret (^) in package.json writes an exact version into yarn.lock. This means that when checking out the branch and running yarn to install the dependencies, yarn.lock will be modified again, leaving the repo in an unclean state:

-"@types/karma@1.7.0":
+"@types/karma@^1.7.0":

This PR fixes that by specifying the dependency's version range instead of the --exact / --tilde switches when installing via yarn. I've tested it on one of my repos with exact, tilde and caret ranges and the lockfile commit always used the correct prefix, resulting in a stable lockfile state.

Please do tell me if I`m missing something that makes using the switches necessary, they seemed like an overly complex way of performing the install to me.

jeysal commented 6 years ago

Regarding logs: This commit in https://github.com/jeysal/ngx-input-flow/tree/greenkeeper-lockfile-test has been created by my fork of greenkeeper-lockfile. The corresponding travis build is https://travis-ci.org/jeysal/ngx-input-flow/builds/282907124

janl commented 6 years ago

Cool, thanks! — I’m out of time for working on this today, but I’ll make sure we address this in the next sprint. o/

noseglid commented 6 years ago

Can we get a resolution to this soon? Every PR we get from greenkeeper breaks our build because it doesn't preserve the caret sign.

jeysal commented 6 years ago

Rebased this to resolve conflicts. Still behaving correctly on my test repo, just as before the rebase.

Is there anything to do before merging this?

jeysal commented 6 years ago

I'll take care of the failing tests asap.

jeysal commented 6 years ago

tests now expect the new yarn commands

jeysal commented 6 years ago

rebased on master.

jeysal commented 6 years ago

If @janl is occupied atm, is there perhaps any other maintainer who could merge this?

aarr0n commented 6 years ago

@janl any progress on this?

aarr0n commented 6 years ago

💚 @janl