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

Fix isString() method in yaml_cpp_adapter #185

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

domire8
Copy link
Contributor

@domire8 domire8 commented Dec 13, 2023

Hi @tristanpenman, @psigen,

First of all, thanks for the amazing work in this library, super cool!

I've tried the yaml_cpp_adapter to validate a YAML file with a custom JSON schema and experience runtime errors when the YAML app included arrays of objects, for example

start:
  test:
    - { name: one, value: two }
    - { name: ten, value: twenty }

and I seem to have found the bug that was causing these runtime errors. While it's true that each element in a YAML should be considered as string (hence probably the return true in the isString() method), this should only return true when the element is actually a scalar value and not a sequence or a map.

Happy to hear your feedback.

@tristanpenman tristanpenman merged commit 5d7a6c5 into tristanpenman:master Dec 14, 2023
0 of 2 checks passed
@tristanpenman
Copy link
Owner

Hey @domire8, thanks for the fix. This seems like a reasonable change to me 👍

@domire8 domire8 deleted the fix/yaml-cpp-adapter branch December 15, 2023 09:10
@psigen
Copy link
Contributor

psigen commented Dec 16, 2023

Thanks for finding and fixing the issue so quickly!

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.

3 participants