-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add mode as development
#1653
Add mode as development
#1653
Conversation
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.
Some notes. Good work!
bin/webpack-dev-server.js
Outdated
|
||
const Server = require('../lib/Server'); | ||
|
||
const addEntries = require('../lib/utils/addEntries'); | ||
const createDomain = require('../lib/utils/createDomain'); | ||
const createLogger = require('../lib/utils/createLogger'); | ||
const createSchema = require('../lib/utils/createSchema'); |
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.
Good idea, maybe better name for this are createConfig
(avoid misleading between schema and configuration)?
lib/utils/createSchema.js
Outdated
const path = require('path'); | ||
const { defaultTo } = require('../../bin/utils'); | ||
|
||
module.exports = function processOptions(config, argv, { port }) { |
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.
Same here createConfig
Codecov Report
@@ Coverage Diff @@
## master #1653 +/- ##
==========================================
- Coverage 82.64% 75.62% -7.03%
==========================================
Files 15 18 +3
Lines 484 603 +119
Branches 120 171 +51
==========================================
+ Hits 400 456 +56
- Misses 67 113 +46
- Partials 17 34 +17
Continue to review full report at Codecov.
|
✅ rename |
"watchOptions": undefined, | ||
}, | ||
"entry": "./app.js", | ||
"mode": "development", |
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.
mode
is added to the config.
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.
👍
Let's wait CI green and merge 👍 |
Only the MacOS Node 11 fails the test. https://travis-ci.org/webpack/webpack-dev-server/jobs/490304902 |
Very weird, because test passed before and all works fine 😕 , maybe we can find what tests fails? |
@evilebottnawi CI is green 🍏 I do not know the cause😵 Thank you for rebuilding. |
@hiroppy any ideas why coverage is dercrease? |
|
👍 |
Great job, thanks for helping 👍 |
Can we release a patch version?😏 |
@hiroppy need minor release other feature will be merged already, i think we can solve couple issues and do release |
fixes #1560
For Bugs and Features; did you add new tests?
yes
Motivation / Use-Case
Separate the place to build the configuration file from
bin/webpack-dev-server.js
in order to make it easier to test.Add
mode
flag asdevelopment
.Breaking Changes
no
Additional Info
I will increase the coverage of the test with other PRs.