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

core: don't catch exception for parse fail #107

Closed
wants to merge 1 commit into from

Conversation

martingkelly
Copy link
Contributor

@martingkelly martingkelly commented Aug 23, 2017

I need to update the tests to not fail on this... will do soon.

When JSON/YAML parsing fails, you currently get a cryptic error
indicating pykwalify was unable to load the file. It is much more
helpful to see the actual parse error so that you can easily fix it. So,
just let the exception through rather than swallowing it.
@coveralls
Copy link

coveralls commented Aug 23, 2017

Coverage Status

Coverage increased (+0.07%) to 87.268% when pulling 0ce39d2 on XevoInc:unstable into e7fac6a on Grokzen:unstable.

@martingkelly
Copy link
Contributor Author

I updated the PR so that the tests pass now. The failing checks in Travis are caused by some issue installing ruamel and seem unrelated to this change.

@Grokzen Grokzen closed this Dec 3, 2017
@martingkelly
Copy link
Contributor Author

Hi, why did this pull request get closed? It looks like several others got closed as well.

@Grokzen
Copy link
Owner

Grokzen commented Dec 3, 2017

@martingkelly oh i missed that O.o i moved away from a master/unstable branch model to only use a master branch. I guess github decided to close all PR:s automatically when i removed the destinatino branch. It looks like i can't really reopen it and change the destination branch to master so you as the author has to do that manually if you want it open again -_- This was not my intent to happen

@martingkelly
Copy link
Contributor Author

Ah, makes sense. I'll rebase and push it again, then reopen the request. It looks like this happened to several other PRs, so you may want to let the other requesters know.

@Grokzen
Copy link
Owner

Grokzen commented Dec 4, 2017

The PR:s that is relevant to do that i will. Some of them i can merge manually and just mention that it was merged.

@Grokzen
Copy link
Owner

Grokzen commented Feb 6, 2018

See #116 for merged code

@Grokzen Grokzen added the Merged label Feb 6, 2018
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.

3 participants