Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Install root dependencies #228

Merged
merged 5 commits into from
Nov 7, 2016
Merged

Conversation

theworkflow
Copy link
Contributor

Install root dependencies when package.json is present. Then moves them to the demeteorized bundle.

Closes #181

if (err && err.code === 'ENOENT') return done();
if (err) return done(err);

FS.copy(source, destination, done);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be called copy-dependencies.js instead? 🙃

@fiveisprime
Copy link
Collaborator

I fixed the build issues and resolved the conflict with master since I broke that..

@theworkflow theworkflow force-pushed the feature/install-root-dependencies branch from 2e29f90 to 6fa8771 Compare November 2, 2016 16:55
Remove unnecessary semicolons so the build passes. I broke it so I figured
I would fix it. <3
@theworkflow theworkflow force-pushed the feature/install-root-dependencies branch from 6fa8771 to bb09432 Compare November 2, 2016 18:08
@theworkflow
Copy link
Contributor Author

Merge conflicts have been resolved.

@jackboberg
Copy link
Contributor

@theworkflow Are you considering closing #243 with this PR?

@theworkflow
Copy link
Contributor Author

@jackboberg Not initially. I had the thought process that each function/file is an individual task. Updating the engines block of the package.json would be a different task. If you feel it should be handled best in this PR I'm down for adding it.

Copy link
Contributor

@jackboberg jackboberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A separate PR seems fine, I think it could be a part of find-node-version.

@theworkflow theworkflow merged commit 0402233 into master Nov 7, 2016
@jackboberg jackboberg deleted the feature/install-root-dependencies branch November 8, 2016 15:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants