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

Fail throwing Error from custom rule #21

Merged
merged 2 commits into from
Nov 20, 2020
Merged

Fail throwing Error from custom rule #21

merged 2 commits into from
Nov 20, 2020

Conversation

mondeja
Copy link
Member

@mondeja mondeja commented Nov 19, 2020

Closes #20

If you throw an Error from a custom rule is not reported, failing silently. I have used the error state in the absence of an exception state, but I'm not that familiar with the implementation as to define a new state because I'm not sure of the implications.

Copy link
Contributor

@birjj birjj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick PR!

I think using the error state is the right call. This means that the exception itself will be displayed as an exception (using the !!! prefix), while the file will be marked as errored. It's been a while since I've worked with svglint though, so it'd probably be best to manually test that this is the case.

I'm happy to merge and release after some minor changes:

src/lib/linting.js Outdated Show resolved Hide resolved
test/custom.spec.js Show resolved Hide resolved
@mondeja
Copy link
Member Author

mondeja commented Nov 20, 2020

Changes done! 👍

@birjj
Copy link
Contributor

birjj commented Nov 20, 2020

👍 Thanks! I've quickly manually tested, and everything seems to be in order. I'll get this released asap.

@birjj birjj merged commit bfbc47a into simple-icons:master Nov 20, 2020
@github-actions
Copy link

🎉 This PR is included in version 1.0.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom rules fail silently
2 participants