Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Implement installations with just a shrinkwrap.json #3628

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

robertkowalski commented Jul 5, 2013

Fixes #3437

Member

robertkowalski commented Jul 5, 2013

ah wait! found a bug!

Member

domenic commented Jul 5, 2013

Looks reasonable, but how about a test? :)

Member

robertkowalski commented Jul 5, 2013

yeah the test i added found that bug :) is fixed now, will update the PR tomorrow.

Member

robertkowalski commented Jul 6, 2013

Ok now!

Member

domenic commented Jul 6, 2013

Very impressive work... unfortunately it's far too much code for me to feel confident reviewing on my own. Tagging for @isaacs to look at.

else cb(er)

@isaacs isaacs commented on the diff Oct 24, 2013

lib/build.js
- ( [ !didPre && [lifecycle, pkg, "preinstall", folder]
- , [linkStuff, pkg, folder, global, didRB]
- , pkg.name === "npm" && [writeBuiltinConf, folder]
- , didPre !== build._noLC && [lifecycle, pkg, "install", folder]
- , didPre !== build._noLC && [lifecycle, pkg, "postinstall", folder]
- , didPre !== build._noLC
- && npm.config.get("npat")
- && [lifecycle, pkg, "test", folder] ]
- , cb )
+
+ var json = path.resolve(folder, "package.json")
+ fs.exists(json, function (exists) {
+ if (!exists)
+ json = path.resolve(folder, "npm-shrinkwrap.json")
+
+ readJson(json, function (er, pkg) {
@isaacs

isaacs Oct 24, 2013

Owner

The fs.exists here is unnecessary. Just start out by reading the package.json, and if it has an error, then try the shrinkwrap. Something like this:

var json = path.resolve(folder, "package.json")
readJson(json, function (er, pkg) {
  if (er) {
    json = path.resolve(folder, "npm-shrinkwrap.json")
    return readJson(json, next)
  }
  next(er, pkg)
})

function next(er, pkg) {
  if (er) return cb(er)
  chain
    ( [ ...
Owner

isaacs commented Oct 24, 2013

I'm apprehensive making changes in cache.js, but I can see the usefulness in this. Putting on the back burner for now, to review again later.

@robertkowalski robertkowalski deleted the robertkowalski:implement/3437-shrinkwrap-json-alone-squashed branch May 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment