clarkie / dynogels

DynamoDB data mapper for node.js. Originally forked from https://github.com/ryanfitz/vogels
Other
490 stars 110 forks source link

Upgrades dependencies #111

Closed clarkie closed 6 years ago

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 98.66% when pulling b5ffacb07d826b0f4fa96974a673b3ed1b26818f on upgrades into 7623ed76d67de525a3e3761846735ad244583105 on master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 98.66% when pulling b5ffacb07d826b0f4fa96974a673b3ed1b26818f on upgrades into 7623ed76d67de525a3e3761846735ad244583105 on master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 98.66% when pulling b5ffacb07d826b0f4fa96974a673b3ed1b26818f on upgrades into 7623ed76d67de525a3e3761846735ad244583105 on master.

clarkie commented 6 years ago

@cdhowie @dangerginger thoughts?

I've added you both as collaborators btw ;)

jkav77 commented 6 years ago

@clarkie I will look at this and get back to you this weekend. Thanks for inviting me to be a collaborator!

cdhowie commented 6 years ago

I'm a bit worried that the use of specific versions in dependencies will cause dependency conflicts with other packages that might do the same thing. Is there some set of best practices we're following that indicates this is preferred?

clarkie commented 6 years ago

Ah yeah, sorry, that's my fault. In our own projects at Tido we pin to specific versions. We got burnt by a public package not following semver so I changed my machine to pin them by default. I'll fix these so they're fuzzy

clarkie commented 6 years ago

Although, actually, they weren't fuzzy before, hmmmm.

clarkie commented 6 years ago

I'm not sure you'd get a conflict as such as an npm install can handle install multiple versions of the same package: e.g. image

cdhowie commented 6 years ago

Right, I forgot that npm does that now.

jkav77 commented 6 years ago

Yeah even though you can install multiple versions of a package doesn't necessarily mean there won't be any problems if the components expose their joi schemas. I think version pinning is a good idea because you can make a PR like this and ensure the tests all pass.

The potential problem would be one dependency creates a v10.6 joi schema and then another dependency tries to use it with only v8.1. I'm not sure that will be the case, but the schema do get created and passed around as components for the dynogels models.

I like the version pinning.

I skimmed this article about npm dependencies.