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

Fix the double plugin bug #146

Merged
merged 1 commit into from
Mar 30, 2017
Merged

Conversation

fatfisz
Copy link
Contributor

@fatfisz fatfisz commented Mar 30, 2017

This reverts the change in 2171200, because it si not covered by any tests. If getRealPath should handle unparsed options, then I'd at least like to add some tests.

@fatfisz fatfisz requested a review from tleunen March 30, 2017 00:59
@tleunen
Copy link
Owner

tleunen commented Mar 30, 2017

I agree the default shouldn't be there, pluginOpts. regExps should always be an array if the code run inside manipulateOptions.

Have you found why the plugin fails for a few people? I didn't get a chance to take a deeper look yet.

@fatfisz
Copy link
Contributor Author

fatfisz commented Mar 30, 2017

I've posted a comment in the bug thread with more details.

The debugging was a bit tedious, lasted a few hours, but at least now I can go to sleep with less worries. G'night :)

@codecov
Copy link

codecov bot commented Mar 30, 2017

Codecov Report

Merging #146 into master will not change coverage.
The diff coverage is 100%.

Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️
src/getRealPath.js 100% <100%> (ø) ⬆️

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 ed54855...8700977. Read the comment docs.

@tleunen
Copy link
Owner

tleunen commented Mar 30, 2017

Thank you so much for the investigation and the help on this one!

@tleunen tleunen merged commit 4e19188 into tleunen:master Mar 30, 2017
@fatfisz
Copy link
Contributor Author

fatfisz commented Mar 30, 2017

This fixes #148.

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.

None yet

2 participants