Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

update artsy-eigen-web-association #8

Merged
merged 2 commits into from
Jan 16, 2019

Conversation

xtina-starr
Copy link
Contributor

Addresses: https://artsyproduct.atlassian.net/browse/GROW-996

This updates the artsy-eigen-web-association model as a part of artsy/artsy-eigen-web-association#23.

@xtina-starr
Copy link
Contributor Author

@joeyAghion this seems a little old to me. I noticed that the node modules seem to be checked in here. I also noticed that there was no yarn.lock or package-lock.json file checked in prior to this pr. I was attempting to simply update the artsy-eigen-web-association package. Is there something I'm missing?

@joeyAghion
Copy link
Contributor

I haven't been that involved in this project, so can't speak expertly. It definitely predates yarn and, as you observed, checks in dependencies. I don't see any problems with the changes in artsy/artsy-eigen-web-association#23, but did you intend to bump all other dependencies as well? If not, might be worth scoping your update to the one library. Either way, it's a simple app so if you were able to confirm that it works locally (just redirecting, basically), that might be enough.

Note that it runs on an old node version as well, 0.12.8 according to opsworks.

@xtina-starr
Copy link
Contributor Author

I didn't intend to bump up all dependencies. The command I ran was npm update artsy-eigen-web-association.

@joeyAghion
Copy link
Contributor

As discussed, I pushed a small update to support a npm run dev command and at least start locally. I hit this local server and got correctly redirected to www.artsy.net, so I imagine this is good to go.

Locally I did see a diff related to the .npmignorein node_modules/artsy-eigen-web-association, but I ignored that and figure it has to do with npm differences.

@joeyAghion joeyAghion merged commit 92e6f16 into master Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants