-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
feat(webpack-cli): add mode argument validation #1290
Conversation
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
dd9840a
to
125096a
Compare
125096a
to
9776d8b
Compare
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.
Please change the test. I we should not say anything about falling back. Webpack already does that and it prints a warning too.
9776d8b
to
e56b3dc
Compare
@snitin315 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
6df4c94
to
430c6ff
Compare
430c6ff
to
cb4e43d
Compare
I think we should add the test case for invalid value , as now we are returning @ematipico what do you say? |
Yes of course. |
@snitin315 I fixed the failing tests. Would you please rebase your PR? |
cb4e43d
to
9a8b9f5
Compare
@ematipico Done 💯 |
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.
We're almost there!
9a8b9f5
to
1827671
Compare
1827671
to
fc4fae5
Compare
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.
Thank you!
What kind of change does this PR introduce?
Fixes #1282
Refers #1261
Did you add tests for your changes?
Yes.
If relevant, did you update the documentation?
Yes
Summary
Added validation for
mode
argument , now can only acceptdevelopment , production or none
will promt a warning if set to any other value.
Does this PR introduce a breaking change?
NO.