-
-
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
BREAKING CHANGE: remove CLI default flag values for webpack-cli serve #2659
Conversation
Codecov Report
@@ Coverage Diff @@
## v4 #2659 +/- ##
==========================================
+ Coverage 92.88% 92.89% +0.01%
==========================================
Files 37 37
Lines 1293 1295 +2
Branches 344 345 +1
==========================================
+ Hits 1201 1203 +2
Misses 88 88
Partials 4 4
Continue to review full report at Codecov.
|
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.
Need fix CI
I just realized that the default behavior I added in
|
9c25995
to
9c1a225
Compare
/cc @hiroppy |
@Loonride Looks something broken on CI |
5d0c8b6
to
996c7cf
Compare
I thought some slight refactoring on the current CLI flags (
|
bin/cli-flags.js
Outdated
name: 'name', | ||
type: String, | ||
describe: 'Name of webpack config to get devServer config from', | ||
}, |
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 we have it here?
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 we have it here?
@evilebottnawi We need it for this: webpack/webpack-cli#1649 (comment). Unless we want to add this flag on the webpack-cli side, but I think it is easiest to keep it uniform with other CLI flags
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.
It is already added on webpack-cli
side
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.
It is already added on
webpack-cli
side
Do you know where to find it on webpack-cli
side? I can't seem to find it
Edit: never mind, I think I found the PR
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.
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.
Yes 👍
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.
/cc @hiroppy
For Bugs and Features; did you add new tests?
Not yet
Motivation / Use-Case
We do not want default values for webpack-cli serve flags, because this makes it much easier to merge CLI flags with the user's devServer options in their webpack config
Breaking Changes
createConfig
util API changesAdditional Info