-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add a test for unexpected failing of validation with multiple units #442
Conversation
No idea why there was no test and why the submissions started failing now... Due to some (weirdly) failing test, I noticed that the check "illegal characters in common definitions" was actually now testing for characters that were sanitized out of common-definition - so I added a quickfix for that (using a character that appears there) on top of the unit-quickfix. |
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.
Two small changes, then good to be merged.
I'd also suggest to rename the PR because you're not just adding a test, you're fixing the multiple unit issue itself.
Co-authored-by: Philip Hackstock <[email protected]>
Thanks for the suggestion, implemented your changes. |
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.
Thanks for implementing my suggestions, good to be merged.
Closes #441