- Notifications
You must be signed in to change notification settings - Fork 282
Build configuration#175
base:master
Are you sure you want to change the base?
Build configuration #175
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nitin42 commented Jun 30, 2017
Untracked changes made it a little difficult to rebase. But anyway I think this is the minimal configuration we require. I will work on reducing the build size for the |
evykassirer left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the delay - still need to look a bit into what the config means, but here's my first pass!
| *.log | ||
| .DS_Store | ||
| .idea | ||
| build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you get rid of .idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I'll make the change.
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "presets": ["es2015"] | |||
| } No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line at end of file pls!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I need to configure prettier settings!
| @@ -0,0 +1,43 @@ | |||
| constwebpack=require("webpack"); | |||
| constpath=require("path"); | |||
| constBabiliPlugin=require("babili-webpack-plugin"); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this isn't used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Yup. We rely on uglifyJS
| }), | ||
| newCleanWebpackPlugin([path.resolve(__dirname,"../build")])// Prebuild (kinda hook) | ||
| ] | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new lineeeee (added to the linter issue)
| @@ -0,0 +1,43 @@ | |||
| constwebpack=require("webpack"); | |||
evykassirerSep 1, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
single quotes please! (I'm surprised the linter didn't catch that, maybe it doesn't check for it - will open an issue)
| NODE_ENV: JSON.stringify("production") | ||
| } | ||
| }), | ||
| newCleanWebpackPlugin([path.resolve(__dirname,"../build")])// Prebuild (kinda hook) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the comments in this file to explain a bit better what things do? kinda hook confuses me
I'm not super familiar with webpack but will look some of this stuff up - but if you could go through this config and explain why you chose this config that'd be good too - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry! I will comment the configuration and will make it easier for you also to make changes afterwards.
| "version": "0.1.6", | ||
| "description": "Step by step math solutions", | ||
| "main": "index.js", | ||
| "main": "./build/mathsteps.min.js", |
evykassirerSep 1, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if build/ is gitignored, will this not work? or will it always build before looking for main
| "test": "node_modules/.bin/mocha --recursive", | ||
| "setup-hooks": "ln -s ../../scripts/git-hooks/pre-commit.sh .git/hooks/pre-commit" | ||
| "build": "NODE_ENV=production ./node_modules/.bin/webpack --config ./config/webpack.config.js --display-max-modules 0", | ||
| "lint": "node_modules/.bin/eslint ./lib", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this removes linting from the test files, which I don't think we should do
evykassirer commented Sep 3, 2017
Thanks :D |
…in42/mathsteps/build-configuration' (PR: google#175)
Added config for
umdbuild.