-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
fix ajv strict warnings in if/else-tests #542
Conversation
@@ -391,8 +391,8 @@ t.test('if/else with const integers', (t) => { | |||
t.plan(2) | |||
|
|||
const schema = { | |||
type: 'integer', |
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.
integer
should be a valid type
. Why change it to number
?
https://json-schema.org/understanding-json-schema/reference/numeric.html#id4
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.
Ajv would complain again.
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.
test/if-then-else.test.js 2> strict mode: type "number" not allowed by context "integer" at "9f255981-9e81-41ed-8a47-ec359679ccd7#/if" (strictTypes)
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.
@climba03003
Does this answer your question?
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.
It does answer my question, but I would like to hear why it is added like this in the beginning.
This test is added in #533
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.
I would like to hear why it is added like this in the beginning
It made some sense before this change. #533 (comment)
It's ok to change it now.
Checklist
npm run test
andnpm run benchmark
and the Code of conduct