-
Notifications
You must be signed in to change notification settings - Fork 28
#487 Implement new sample override #488
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
base: main
Are you sure you want to change the base?
Conversation
|
@thorehusfeldt can you check the schema again? ^^' |
mpsijm
left a comment
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.
Quick code review, didn't test it yet. The CUE schema looks good on first glance (maybe I'd put "ans" before "ans.statement" for consistency, but that's a nit). Note that the JSON schema should also be updated.
|
@mpsijm @thorehusfeldt can you check the schemas? |
bapctools/resources/support/schemas/generators_yaml_schema.json
Outdated
Show resolved
Hide resolved
|
@mpsijm can this be merged? |
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.
Other than that, all is looking good to me! If the schema change was intentional and the discussion in #487 is concluded, feel free to merge 🙂
…uplicate link Pattern fields only result in `"additionalProperties": true` in the JSON schema, which is not precise enough.
|
@mpsijm i changed the logic here a bit, do you want to take a look again? |
mpsijm
left a comment
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.
Still looking good to me 😄 I see you did not (need to) change the tests to match the modified logic in the last commit, which to me means that the external interface has not changed 👍 We may want to add some extra assertions (like verifying that the linked files are actually symlinks) but that may be more effort than it's worth, I'll leave that up to you 🙂
Closes #487.
Try to implement new sample data behaviour