-
Notifications
You must be signed in to change notification settings - Fork 347
"breaking change" in json 2.9.1->2.10.0? #754
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
Comments
Yes it was intentional: #729
It isn't. The exception class is, but the exception message is intended for humans and can change in any version. Heck, up until I fixed it a couple years ago, it was changing on every versions (#470). They're also not consistent between different implementations of the gem (Java, Pure, etc). Also I have further plans to improve these messages, like including line a column numbers, so it might break again. So it's probably best if your test only assume a specific error class, but doesn't try to match the message content. |
json 2.10.0 updated the text in exceptions. This was done in ruby/json#754. metadata-json-lint prints the exception and our test verified the exception text. That's no longer consistent between oder and newer json messages. I think it provides no benefit to match the exception text, so lets remove it from our test.
Thanks for clarification. I just removing matching for the exception text in our tests. |
json 2.10.0 updated the text in exceptions. This was done in ruby/json#754. metadata-json-lint prints the exception and our test verified the exception text. That's no longer consistent between older and newer json messages. I think it provides no benefit to match the exception text, so lets remove it from our test.
Hi,
I'm one of the maintainers of https://github.com/voxpupuli/metadata-json-lint, where we validate a JSON file for the Puppet ecosystem. The relevant code (I think so, at least):
We've some tests in the module, they pass with json 2.9.1 and older. One of the tests assume the following error:
With json 2.10.1 we get:
This could be considered as a breaking change, if we assume the output is even something like a stable/public API. Can you tell me if the rewording was intentional?
The text was updated successfully, but these errors were encountered: