newhavenio / newhavenio.github.io

active version of the website for newhaven.io built on the Jekyll framework
http://newhavenio.github.io/
MIT License
13 stars 12 forks source link

moved bourbon and susy to regular dependencys #69

Closed bsutt123 closed 6 years ago

bsutt123 commented 6 years ago

Bourbon and Susy are both referend in main.scss and Travis was complaining that it didn't have them, so I moved them to regular dependencies.

If someone who knows more about travis knows that it needs/doesn't need webpack in the regular dependencies, then I can move that as necessary as well.

bsutt123 commented 6 years ago

@NerdsvilleCEO So we got a build failure because we had some dependencies in devDeps that needed to be in dependencies (It was bourbon and susy which get imported with sass).

I moved those up, but I'm wondering if you know if TravisCI needs us to include webpack or babel or some of the other devDeps in regular Deps so that it will be able to install them and run the build process.

jnimety commented 6 years ago

travis should do whatever we tell it to do, so maybe just tell it to install dev dependencies since they're required for the build step? @NerdsvilleCEO and @treznick please weigh in since you set that up.

treznick commented 6 years ago

@jnimety @bsutt123 we can test that assumption by removing the dev dependencies and seeing if it builds in a PR. I'll do that

On Sat, Apr 28, 2018 at 10:08 AM, Joel Nimety notifications@github.com wrote:

travis should do whatever we tell it to do, so maybe just tell it to install dev dependencies since they're required for the build step? @NerdsvilleCEO https://github.com/NerdsvilleCEO and @treznick https://github.com/treznick please weigh in since you set that up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/newhavenio/newhavenio.github.io/pull/69#issuecomment-385178704, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlwA0iKip54xC723ABIGd5ApGo3UZAmks5ttHfzgaJpZM4TnWT6 .

jnimety commented 6 years ago

maybe in build.sh we need a yarn install?

jnimety commented 6 years ago

it makes perfect sense that you need dev deps to build/deploy, so we just need to figure out where to have travis install them.

jnimety commented 6 years ago

thanks @treznick !

treznick commented 6 years ago

see #77

treznick commented 6 years ago

FWIW travis will run yarn if it sees a yarn.lock in the repo https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#Travis-CI-supports-yarn

treznick commented 6 years ago

so I think we can just merge this. merging

jnimety commented 6 years ago

Why is yarn install not installing dev deps? On Sat, Apr 28, 2018 at 10:14 AM Tom Reznick notifications@github.com wrote:

Merged #69 https://github.com/newhavenio/newhavenio.github.io/pull/69.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/newhavenio/newhavenio.github.io/pull/69#event-1600163248, or mute the thread https://github.com/notifications/unsubscribe-auth/AAETVMVeypowJyQ6KYHLONtyXoFOKBvqks5ttIdJgaJpZM4TnWT6 .

treznick commented 6 years ago

@jnimety it is. the problem isn't that the build script isn't doing that, and bourbon and susy were listed as devDependencies

The new problem is that our references to bourbon were looking in node_modules. I think there's a webpack configuration option to deal with this.

bsutt123 commented 6 years ago

Of course, I checked all the html and css but didn't check the node_modules references that were there before I switched us over to yarn.

I think that we can require stylesheets with require('susy/bootstrap/...) in the entry,js file. We might need to modify the webpack configu to have a sass loader for us, but that shouldn't be too hard.

I'll work on this once I get back into town on Wednesday if it isn't worked out by then.