Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Update dependencies Greenkeeper found #1703

Closed
ianb opened this issue Oct 5, 2016 · 10 comments
Closed

Update dependencies Greenkeeper found #1703

ianb opened this issue Oct 5, 2016 · 10 comments
Assignees
Milestone

Comments

@ianb
Copy link
Contributor

ianb commented Oct 5, 2016

I closed its first PR, #1642, because the React upgrades seemed to break the build. But it had many dependencies that could be easily updated.

@ianb ianb added this to the Altair milestone Oct 5, 2016
@ianb
Copy link
Contributor Author

ianb commented Oct 10, 2016

At this moment I think it would make sense to just upgrade everything, and then work out any build problems. npm outdated gives the information.

@fzzzy
Copy link
Contributor

fzzzy commented Oct 10, 2016

I agree. What problems were you seeing with the react upgrade?

@ianb
Copy link
Contributor Author

ianb commented Oct 10, 2016

Browserify wasn't able to find some file, after doing a clean build (rm -rf node_modules ; make clean) – I didn't look into it further.

@fzzzy fzzzy self-assigned this Oct 10, 2016
@fzzzy
Copy link
Contributor

fzzzy commented Oct 10, 2016

That's a different issue that is already in master. For a while now I have had to do rm -rf node_modules; rm -rf build; npm install; make all to do a clean build.

@ianb
Copy link
Contributor Author

ianb commented Oct 10, 2016

The clean build wasn't the problem – rather the build problem only showed itself when I did a clean build.

@fzzzy
Copy link
Contributor

fzzzy commented Oct 10, 2016

Yes, the problem was that the node_modules weren't installed before running make all. This problem is in master. I just verified by updating all the modules; I can recreate the problem by running make all without a node_modules, but if I delete node_modules and build, followed by a npm install, and a make all, it works fine with all dependencies updated.

@ianb
Copy link
Contributor Author

ianb commented Oct 10, 2016

Maybe we need all: npm addon server in the Makefile then (though npm is the first dependency of the other two rules, so it seems superfluous)

@fzzzy
Copy link
Contributor

fzzzy commented Oct 10, 2016

Sure, but that is a different bug that should be filed. It has nothing to do with updating the deps.

@ianb ianb closed this as completed in 6a1b6cd Oct 10, 2016
ianb added a commit that referenced this issue Oct 10, 2016
r? ianbicking Fixes #1703 Update all deps to latest.
@ianb
Copy link
Contributor Author

ianb commented Oct 10, 2016

I think the Makefile uses things that get npm-installed just to setup the basic environment, so in practice npm install just has to be run before make. The scripts ensure this at least. I don't think there's anything else worth fixing there.

@fzzzy
Copy link
Contributor

fzzzy commented Oct 10, 2016

Sounds good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants