-
Notifications
You must be signed in to change notification settings - Fork 21
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 duplicate error messages issue #33
Conversation
360f95d
to
e5ad07c
Compare
Awesome work tracking this down and making a fix! I saw that mocha has a Regardless, even though this is a workaround as you said, I'm fine with merging this since it's not practical to force everyone to use that mocha option (if it even works). Looks like you've fixed the tests. Good to go on your end? |
@@ -31,7 +31,7 @@ function test(p, opts) { | |||
) { | |||
throw new Error( | |||
chalk.red('Code did not pass lint rules') + | |||
formatter(report.results) | |||
formatter(report.results).replace(process.cwd() + '/', '') |
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.
Actually, do you think you could add a small comment to explain why the replace is needed? I can see this causing some head-scratching in the future.
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.
sure, done
This is a workaround for mochajs/mocha#2126 but since relative paths are usually much nicer to look at it also has its own advantages
e5ad07c
to
03472e5
Compare
yes, I think this should solve the issue. might be good if you could verify that everything else still works properly though. |
Fix duplicate error messages issue
Thanks! |
This PR resolves #29