-
Notifications
You must be signed in to change notification settings - Fork 3k
Fixed lockfiles not being added to the "npm version" commit #17742
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a small mostly-stylistic change
lib/version.js
Outdated
], options) | ||
} | ||
}).nodeify(cb) | ||
} | ||
|
||
function stagePackageFiles (localData, options) { | ||
return new BB(function (resolve, reject) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need a promise constructor here. Just:
return addLocalFile('package.json', options, false).then(() => {
if (localData.hasShrinkwrap) {
return addLocalFile('npm-shrinkwrap.json', options, false)
} else if (localData.hasPackageLock) {
return addLocalFile('package-lock.json', options, false)
}
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doing this right now! Is there a possibility that this could be merged to the 5.3.0 relese today?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably! I'll take a look in the AM and see if this patch is ready to merge. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
Should be ready for release :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good, everything seems to work. gj, and thanks for the contrib! 🎉
* Sequential promises instead of concurrent * Fixed style errors * Removed the promise constructor PR-URL: #17742 Credit: @markpeterfejes Reviewed-By: @zkat
Hey!
This small PR fixes #17707 by executing the
git add
commands sequentially instead of concurrently, which sometimes causes the aforementioned error.I'm not really sure that this is the prettiest way to implement this, change suggestions are very welcome!
Thank you!