Closed iainbeeston closed 8 years ago
Does anyone want to take a look at this?
Seems alright :+1:
@RST-J Thanks! Do you think it's safe to merge?
I think with this and #255 it's worth making a minor release
I think so, the changes make exception handling more specific which is what we wanted here and as the tests pass, there is at least no break with expected behavior in the non-error cases.
But as there are changes in how errors must be handled I suggest to add an note in the changelog and maybe also hints to the readme about how to deal with schema load errors and the ne JSON parse error before we make a release.
Good idea.
Incidentally, we don't have a changelog, do we? (I was thinking that we should add one starting with v3 but maybe there's no reason to wait?)
I think we could add it right away, there is no reason to wait.
I'd very much appreciate both a minor release with this, #255, and a changelog. :+1: :smile:
Only rescue errors explicitly
There are a lot of places in the code where we rescue any error, at all. That's quite dangerous, as it could conceal bugs.
I've changed it so that we always specify which error class we want to catch. Mostly it's been a minor change, but there are two API changes: