Skip to content

add validations for simple bounds checks - #515

Merged
mattpolzin merged 4 commits into
mattpolzin:mainfrom
true-false-maybe:514-add-validations-for-simple-bounds-checks
Sep 14, 2026
Merged

mattpolzin merged 4 commits into
mattpolzin:mainfrom
true-false-maybe:514-add-validations-for-simple-bounds-checks

Conversation

@true-false-maybe

Copy link
Copy Markdown
Contributor

closes #514

@mattpolzin

Copy link
Copy Markdown
Owner

These seem like nice fits, thanks! I haven't looked at this in detail yet, but before I do, what are your thoughts on combining any of these vs. not? Do you think it's likely to be useful to e.g. validate that integer bounds are sane but not validate that other number bounds are sane?

@true-false-maybe

true-false-maybe commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor Author

These seem like nice fits, thanks! I haven't looked at this in detail yet, but before I do, what are your thoughts on combining any of these vs. not? Do you think it's likely to be useful to e.g. validate that integer bounds are sane but not validate that other number bounds are sane?

it didn't really cross my mind tbh. i basically split them just because i thought it was a nice separation of concerns during implementation and testing (and probably reviewing). if you have a specific set of the added validations you want to be combine into one, i'd implement it. 👍

@mattpolzin

Copy link
Copy Markdown
Owner

Let's combine the number and integer checks then. Leave the others separate. The integer and number checks operate on the same properties (by name) and are conceptually the same as each other so since I don't see someone wanting to check one but not the other I think it makes sense to combine them.

@true-false-maybe

Copy link
Copy Markdown
Contributor Author

combined integer and number checks into one (i kept both test-variants though, as they actually caught a bug while i refactored, so yeah)

@mattpolzin mattpolzin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

Comment thread Sources/OpenAPIKit/Validator/Validation+Builtins.swift Outdated
@mattpolzin
mattpolzin merged commit e68732a into mattpolzin:main Sep 14, 2026
12 checks passed
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

Successfully merging this pull request may close these issues.

Add validations for simple bounds checks for JSONSchema

2 participants