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
Bugfix#344 report error when one of schema doesnt have the discriminator property #346
Conversation
I don't think we need to add this validation in the reference resolver itself. In general, we try to avoid adding a validation into the reader and resolver unless we have to (e.g. YAML malformatting can't really be validated anywhere else). This seems like it would fit better in the Validations Rules https://github.com/Microsoft/OpenAPI.NET/tree/master/src/Microsoft.OpenApi/Validations/Rules This rule belongs to OpenApiSchemaRules. #Closed |
Okay, I will move the validation into OpenApiSchemaRules. Thanks. #Closed |
…doesnt-have-the-discriminator-property
Thanks for adding the tests to the reader. Could you help move the tests to OpenApiSchemaValidationTests.cs? Also, the tests should be adjusted to simply read the Schema object instead of the real document. #Closed |
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.
🕐
…he-discriminator-property' of https://github.com/senthilkumarmohan/OpenAPI.NET into bugfix#344-Report-error-when-OneOf-schema-doesnt-have-the-discriminator-property
Thanks for the review. I have moved test cases to OpenApiSchemaValidationTests (with changes) and updated code as per the comments. #Closed |
test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDiscriminatorTests.cs
Outdated
Show resolved
Hide resolved
Thank you for making this change. I have left a few minor comments. Once you have fixed that and merged in the latest master, we should be good to go :) Edit: Looks like merge is not needed given no conflicts. Github wasn't updating correctly for me. |
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 have updated the resource keys and reverted changes to OpenApiDiscriminatorTests. Thanks again. 👍 |
This looks good now. I'll check it in. Thanks a lot for your contribution :) |
Thankyou very much |
No description provided.