Skip to content
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

Added option 'emitWarnings' to identify unprocessed files #150

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Root-Core
Copy link

Hello there,

as we have two options, which will prevent processing of files, we should have an option to identify these unprocessed files.

You might consider to default this to true or warn without the option, as it should be useful in most cases.
I could do this, if you give me some feedback. 👍

Regards!

@codecov
Copy link

codecov bot commented Dec 13, 2017

Codecov Report

Merging #150 into master will decrease coverage by 6.25%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
- Coverage     100%   93.75%   -6.25%     
==========================================
  Files           1        1              
  Lines          28       32       +4     
  Branches       10       12       +2     
==========================================
+ Hits           28       30       +2     
- Misses          0        2       +2
Impacted Files Coverage Δ
index.js 93.75% <60%> (-6.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1459fd...2a6ccf8. Read the comment docs.

@bhovhannes
Copy link
Owner

Thanks for your PR!

Code looks great. Can you add some tests to make codecov happy?

Also I am curious if it is possible to see the name of the file in the warning message. Message will be much more informative if it will contain the name of the file for which svg-url-loader felt back to file-loader.

@Root-Core
Copy link
Author

I can look into it, tough I dont't really know how this could be tested.

The file name seems to be unavailable and not necessary too, as webpack shows warnings emited via this.emitWarning() straight under the regarding file name + some additional info.
At least it had done it for me, while testing this. I could provide screenshots if you want.

@bhovhannes
Copy link
Owner

@Root-Core , then add some tests and I'll be happy to merge your PR.

Regarding tests. Perhaps it will be possible to setup a spy on emitWarning() method.

Base automatically changed from master to main February 28, 2021 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants