-
Notifications
You must be signed in to change notification settings - Fork 0
feat: support required
fields (make the others optional)
#17
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
Conversation
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
related dylibso/mcp.run-servlets#59 |
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.
This looks correct to me but maybe let's have @nilslice chime in
IIRC @nilslice mentioned we'll need to release https://github.com/dylibso/xtp-bindgen-test/ for the CI to pass |
oh yea, feel free to release the dependent library and re-run these tests. Don't let me be a blocker on these, if the tests pass we can push it along |
it looks like the test is failing
either the test or the mock... 🤔 |
what's the status here? I keep getting reminders but don't know who's on this now (re: test suite etc) |
it's still me I guess |
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
55a39ec
to
4418c74
Compare
ok making some progress here
|
@evacchi - ok thanks, can you be sure to link this issue to the GitHub ticket to track the JS PDK bug with whatever info you have on it? |
@zshipko fixed the issue, we can merge this and close |
isZigOptional(schema, property)
, checking whether the field is nullable OR it is not declared as required in the parent schema.requires dylibso/xtp-bindgen-test#28
Result, e.g.: