busterjs / buster

Abandoned - A powerful suite of automated test tools for JavaScript.
http://docs.busterjs.org
Other
448 stars 37 forks source link

Freeze dependencies using npm-shrinkwrap.json. #469

Closed mroderick closed 6 years ago

mroderick commented 8 years ago

Add script/shrinkwrap-pre-commit to automatically update npm-shrinkwrap.json, whenever package.json changes.

This fixes #464

dominykas commented 8 years ago

Does this mean that there's a setup step on dev side? Could this work with, say as part of preversion npm script? Or maybe as a postinstall (but only if done locally by dev: https://www.npmjs.com/package/in-publish)?

dominykas commented 8 years ago

Hmmm. Or even better - prepublish - it would then also work nicelly and automagically when publishing from Travis (I intend to set that up soon).

mroderick commented 8 years ago

Does this mean that there's a setup step on dev side?

They're already doing this step with npm install

mroderick commented 8 years ago

When you want to freeze dependencies is entirely up to you. The longer you wait after updating package.json (and running all the tests), the more likely dependencies are to change and produce surprising results.

The context for the changes is clearest at the time of change, when a contributor is trying to make a change work. Weeks or months later, that context is likely to be less clear to the person charged with publishing a new version. Thus, it's a lot easier to resolve any issues at the time of making the changes.

dominykas commented 8 years ago

Oh I completely agree that it's best to do this as soon as deps are updated, i.e. freeze when you test. To rephrase my question - if I understand correctly, this script needs to be added as a git pre-commit hook, right? Or something like https://www.npmjs.com/package/precommit-hook has to be used? If so - there's space for dev error, so I wonder if it there's a way to do this automagically in CI (i.e. after all tests pass)?

Another thing I'm pondering is http://greenkeeper.io - I haven't tried it on anything yet, but it seems that it adds exact versions in package.json and submits a PR - which means Travis automatically tests the updates. But even so - there's still a need for shrinkwrap, since the deeper deps might change (greenkeeper only tracks direct deps) - but I don't think greenkeeper will update shrinkwrap (= manual step required by the dev), so maybe there's a way to run updates as part of npm lifecycle, so that no setup is required?

mroderick commented 8 years ago

The pre-commit dependency fixes the need for developers doing anything other than npm install

dominykas commented 8 years ago

Sorry, I'm confused a little :( The developer still needs to install the hook itself, correct?

mroderick commented 8 years ago

Sorry, I'm confused a little :( The developer still needs to install the hook itself, correct?

The hook will be installed by the pre-commit module during npm install

dominykas commented 8 years ago

Aha! Got it now :)

Does it also ensure that all the pkgs are matching the previous shrinkwrap/are at their latest version? E.g. if I update some pkg, you git pull, but do not 'npm i' - our trees would be differrent?

I'll merge this next week as I get back to somewhat more normal schedule.

mroderick commented 8 years ago

It only ensures that whenever you change package.json, that you also update npm-shrinkrwrap.json. I can add a post-receive hook that runs npm install whenever you receive an updated npm-shrinkwrap.json file, if you want.

dominykas commented 8 years ago

Nah, that would probably do more harm than use.

dominykas commented 8 years ago

Can't merge - "This branch has conflicts that must be resolved" - can you please rebase?