open-austin / budgetparty

Budget Party was built to help people understand and augment a city budget for Austin. It is an interactive app that is best used in context of a "Budget Party" event.
https://austinbudget.party
The Unlicense
25 stars 24 forks source link

Up-and-running changes #209

Closed danielahedges closed 6 years ago

danielahedges commented 6 years ago

I had to make some minor changes to get up-and-running. The biggest problem was a case sensitivity bug (see eb55a00), which meant I couldn't launch on my Mac. The others were mostly just things I thought would help.

Thanks! --Dan

twentysixmoons commented 6 years ago

Thanks @danielahedges @mateoclarke can you review this when you get a chance?

danielahedges commented 6 years ago

The change inside package.json was largely moving dependencies into devDependencies. If you just do an npm install, you install both dev and regular dependencies. So you will not see a change.

The reason it is preferable to make react and redux regular dependencies is because this web server will always need them, even when you have a minimal container. devDependencies are meant to be stripped out when you run an npm install --production, or do an npm prune --production.

It's my recommendation that you move run-time dependencies out of the devDependencies section. I usually only populate devDependencies with packages needed only for:

  1. building
  2. testing
  3. developing (like linting)

So, to sum up, I agree right now you don't need the package.json changes, but you will likely want them in the future, and they are not doing any harm now.

Hope that helps!

danielahedges commented 6 years ago

According to this thread: https://stackoverflow.com/questions/9268259/how-do-you-prevent-install-of-devdependencies-npm-modules-for-node-js-package

When I said npm install --production, that command has been deprecated, and instead we now use npm install --only=production, but the sentiment remains the same.

mateoclarke commented 6 years ago

Thanks for this PR @danielahedges. I had to resolve a merge conflict so I merged commits into the codebase in a separate PR #214.

mateoclarke commented 6 years ago

thumbs_up