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

Misleading results for @exodus/schemasafe #52

Closed
jchook opened this issue Nov 19, 2020 · 1 comment
Closed

Misleading results for @exodus/schemasafe #52

jchook opened this issue Nov 19, 2020 · 1 comment

Comments

@jchook
Copy link

jchook commented Nov 19, 2020

Hi, thanks very much for putting this benchmark together.

I found this repo very helpful in evaluating which JSON schema validator to use in my latest project. However, I found the benchmarks for @exodus/schemasafe misleading.

schemasafe disables error reporting by default, which seems like significantly different (and unexpected) behavior compared to the other validators in this benchmark.

I did see this note in CONTRIBUTING.md:

Also, do not tweak any of the settings or options of a validator. Each validator should be run in it's standard setting. This is to make sure that nothing breaks if the benchmarks changes. But also to reflect the kind of performance a user of the validator would get, if using it without tweaking.

However, in my opinion, to make an "apples to apples" comparison between schemasafe and the other validators, we should apply the includeErrors option to schemasafe.

@ebdrup ebdrup closed this as completed in 011e2fd Nov 29, 2020
@ChALkeR
Copy link
Contributor

ChALkeR commented Sep 14, 2021

Yeah. This wasn't done on a purpose, I mentioned this back when I added schemasafe: #46 and proposed to enable includeErrors there.

Glad that this is resolved now ).

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

No branches or pull requests

2 participants