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

Changed markup to plain html, added linting to npm scripts; added editorconfig file #37

Merged
merged 8 commits into from Sep 26, 2016

Conversation

tarnas14
Copy link
Collaborator

configured bootstrap with webpack (didn't google if there's a new, better way, probably should check)

@@ -23,6 +24,20 @@
"babel-preset-es2015": "^6.6.0",
"babel-preset-react": "^6.5.0",
"babel-preset-stage-0": "^6.5.0",
"bootstrap": "^3.3.7",
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like dependencies and devDependencies are mixed up.
Everything that would be needed to build the app on production should listed as "dependencies" node. Things that are not for production like mocha should be listed as "devDependencies".
webpack/webpack#520

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

true

Copy link

Choose a reason for hiding this comment

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

shouldn't mocha also be in the dependencies to allow for
export NODE_ENV=production && npm i && npm test to be able to run tests using mocha under production environment? Otherwise you would have to remember (and the CI system) to set NODE_ENV to development before npm i

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fair point! I'm not sure if that is a common practice, to be honest
on production environment I'd go for running integration/smoke tests that treat the application as a black box
but if they treat it as a black box and use public api, it doesn't matter where they're run, they just need access to a running app
also, we don't have any tests here which is a whole other story

filename: './src/public/[name].js'
path: path.resolve(__dirname, './src/public/'),
filename: '[name].js',
publichPath: '/public/'
Copy link
Owner

Choose a reason for hiding this comment

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

publich -> public and then: why did it work? or it didn't?

Copy link
Collaborator Author

@tarnas14 tarnas14 Sep 25, 2016

Choose a reason for hiding this comment

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

it looked ok oO, maybe only because we're not using glyphicons or something

@kjendrzyca kjendrzyca merged commit 5b9e7db into kjendrzyca:master Sep 26, 2016
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.

None yet

3 participants