search

metalsmith / permalinks

A Metalsmith plugin for permalinks.
MIT License
62 stars 67 forks source link

Properly disable package-lock.json. #103

Closed XhmikosR closed 5 years ago

XhmikosR commented 5 years ago

@Ajedi32 note that I'm personally against this but since there's no lock file this is the proper way to disable it.

Ajedi32 commented 5 years ago

Yeah, I'm not sure why that was ignored in the first place. Here's the commit that introduced it: https://github.com/segmentio/metalsmith-permalinks/commit/f91627ef2f5ec0aa0d0703f564be1031e460ac54#diff-a084b794bc0759e7a6b77810e01874f2R37

Though I suppose an argument could be made that it makes sense to test with the dependencies your users will be installing, rather than just whatever happens to be in the package lock file... not really sure what the best thing to do is here.

XhmikosR commented 5 years ago

TBH I always include a lock file, and use npm ci in CI. I'd say let's merge this so that we keep the previous behavior but make it proper.

Ajedi32 commented 5 years ago

Alright, I'll go ahead and merge this for now.

Long term, it might be better to move to a different model where we lock dependencies in dev, but take additional steps to insure tests are run with the latest versions of dependencies (like maybe a separate test check in CI which deletes package-lock.json before installing dependencies. Or maybe a bot that constantly submits PRs with an updated package-lock.json and warns if anything breaks. That can be done with future changes. For now I agree it's probably best to just codify the current behavior.