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

Fix heroku build #47

Merged
merged 5 commits into from
Apr 10, 2017
Merged

Fix heroku build #47

merged 5 commits into from
Apr 10, 2017

Conversation

dpikt
Copy link
Contributor

@dpikt dpikt commented Apr 6, 2017

Node-sass was giving us trouble - we have to force a rebuild using yarn remove.
yarnpkg/yarn#1832

Also:

  • Made yarn run storybook:build a heroku-specific script
  • Made sass a dev dependency
  • Removed prepublish linting

@dpikt dpikt requested a review from davemcorwin April 6, 2017 21:52
@dpikt dpikt temporarily deployed to lp-components-pr-47 April 6, 2017 21:52 Inactive
@dpikt dpikt changed the title Fix heroku builds Fix heroku build Apr 6, 2017
@davemcorwin
Copy link

weird. So looking at the package.json

  1. shouldn't documentation and jest be devDependencies?
  2. what is the sass module?
  3. have you seen this issue consistently even after clearing the npm cache?

@dpikt
Copy link
Contributor Author

dpikt commented Apr 7, 2017

  1. Yes
  2. I don't know, let me see if it compiles without it
  3. Yes. I don't think Heroku is using the yarn cache anyway.

@dpikt
Copy link
Contributor Author

dpikt commented Apr 7, 2017

@davemcorwin switched up the dependencies. I removed sass since it wasn't doing anything- I probably mistakenly thought it was a peer dependency of node-sass.

Copy link

@davemcorwin davemcorwin left a comment

Choose a reason for hiding this comment

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

word

@dpikt dpikt requested a deployment to lp-components-pr-47 April 10, 2017 17:16 Pending
@dpikt dpikt merged commit 5251585 into master Apr 10, 2017
@dpikt dpikt deleted the fix-heroku branch April 10, 2017 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants