-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fixed warnings not being logged to console #26
Conversation
Warnings were not being logged unless there was an error in the css linting. Fixes webpack-contrib#23
Thanks for the PR @iblack10, it looks reasonable to me :) would you mind adding a fixture that would provide a warning and a test that ensures the warnings are printed? |
Hope that's OK - seemed that the plugin wasn't actually carrying any warnings through from stylelint. |
This will need to be rebased before merging :) |
@@ -49,5 +55,8 @@ module.exports = function runCompilation(options, compilation, done) { | |||
errors.forEach(function (err) { | |||
compilation.errors.push(err); | |||
}); | |||
warnings.forEach(function (warning) { | |||
compilation.warnings.push(warning); |
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 use the .concat
method instead of going through each warning. I've been meaning to fix the original code as well so if you could change it that'd be great.
Looking at this further I think there are some problems with the tests. Test 8 should actually have zero errors but it's reporting 1. Seems like the .map function means that even if there are no errors or warnings it will return the source of the file being tested resulting in the length of the error/warning array never being zero. I'll take another look tomorrow. |
I'm hopeful that's everything sorted now. Sorry there are so many commits inside of this pull request - pretty new to Github! |
Thanks @iblack10 this is great so far. If there are other cases you could add with other fixtures, could you add them or let me know otherwise? I'll merge after that :) |
Great, I think that's all for now - it's working well for me! |
* Fixed warnings not being logged to console Warnings were not being logged unless there was an error in the css linting. * Corrected number of warnings returned
Warnings were not being logged unless there was an error in the css linting.
Fixes #23