Webpack verbose #401
Webpack verbose #401
Conversation
|
|
||
| var webpackDefaults = { | ||
| var webpackVerbose = { | ||
| stats: { |
There was a problem hiding this comment.
Now that verbosity is hidden behind a flag, It might be better to rely on some more verbose presets that webpack offers.
What about stats: "verbose" orstats: "normal"?
| @@ -17,9 +20,10 @@ var webpackDefaults = { | |||
| module.exports = function packageServerScript(params, callback){ | |||
| var fileName = 'server.js'; | |||
| var publishPath = params.publishPath; | |||
| var webpackParams = params.verbose ? webpackVerbose : webpackDefault; | |||
There was a problem hiding this comment.
Why not doing all this logic at the time you define the webpackParams?
i.e:
webpackParams = {
stats: params.verbose ? 'verbose' : 'errors-only'
}|
|
||
| var packageOptions = { | ||
| componentPath: dir, | ||
| minify: false, |
There was a problem hiding this comment.
What does minify do ?
There was a problem hiding this comment.
minification of js/css static assets
There was a problem hiding this comment.
Isn't the PR for webpack verbosity? Or the PR scope changed?
There was a problem hiding this comment.
Correct. I changed the signature there, not altering the behaviour. The false there was for minify
| @@ -12,12 +12,10 @@ var getUnixUtcTimestamp = require('../../utils/get-unix-utc-timestamp'); | |||
| var validator = require('../../registry/domain/validators'); | |||
|
|
|||
| module.exports = function(){ | |||
| return function(componentPath, minify, callback){ | |||
There was a problem hiding this comment.
I changed this signature to accept an options parameter instead of adding another one
There was a problem hiding this comment.
I was wondering if we should have a minify option at all, but probably I'm missing some context. Why would we want to set minify, what are we trying to solve?
There was a problem hiding this comment.
Minify is been there for a long time. It is not referred to server.js, not to template.js, but to actual static contents in the static folders. It is set component based on the package.json, and we use it for some components at opentable. I can show you some examples on Monday.
Fixes #377
I tested it locally and seems to work nicely :)
Can you have a look at it @nickbalestra?