-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
No errors plugin deprecate #3570
Conversation
…zopos/webpack into no-errors-plugin-deprecate
@TheLarkInn saw you add the low-hanging-fruit label to this yesterday, thought I'd give it a go. |
… check that there is no warning when using it
@@ -0,0 +1 @@ | |||
require('./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.
Write a test case with error.
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.
Will do!
new webpack.NoErrorsPlugin() | ||
] | ||
}, function(errors, warnings) { | ||
if(errors.length === 0) { |
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.
These conditions are weird. A test should have an expected behavior and not two possible.
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 simply took what was above in the same file! I do agree they don't really make sense though.
@sokra all patched up. |
@@ -0,0 +1,3 @@ | |||
module.exports = function a() { |
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.
These files should not be commited. They are only create temporary by the test suite. You probably canceled tests before they are remove. If you want to you could add them to .gitignore
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.
Yeah, canceled before test suite was entirely done. Will remove them and ignore them as well. Thanks @sokra!
Not entirely certain what the failed check is about. |
Thanks |
webpack deprecated NoErrorsPlugin in favour of more expressively named NoEmitOnErrorsPlugin: webpack/webpack#3570
NoErrorsPlugin is deprecated by NoEmitOnErrorsPlugin. (webpack/webpack#3570)
What kind of change does this PR introduce?
It moves current NoErrorsPlugin functionality to a more accurately named NoEmitOnErrorsPlugin.
Did you add tests for your changes?
I did.
Summary
I saw an issue filed #3179.
I also think that the new plugin name will make things less confusing for users, ultimately resulting in less open issues. The deprecation warning also helps ease the transition.
Does this PR introduce a breaking change?
It does not. It does provide a warning however that in the future there might be a breaking change coming.
Other information
My first contribution - hopefully first of many. I want to acquire innate knowledge of THE best build tool ever and what better way to do it then start off with some bugs / small enhancements? :)
Double commit because I amended commit message after reading contribution guidelines.