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

ISSUE-1131: Added diagnostic reporting recursive concatenation of dia… #1245

Merged
merged 3 commits into from
Jun 13, 2023

Conversation

elize-vdr
Copy link
Contributor

Added diagnostic reporting recursive concatenation of diagnostic reports from referenced files to the main file's diagnostic report.
Also added a unit test in C:\Users\Elize.van.der.Riet\Work\OpenAPI\OpenAPI.NET - Fork\OpenAPI.NET\test\Microsoft.OpenApi.Readers.Tests\OpenApiReaderTests\OpenApiDiagnosticTests.cs, my apologies not sure if this is the right place for the test, please let me know if I should move it.

…gnostic reports from referenced files to the main file's diagnostic report
@darrelmiller
Copy link
Member

Thank you for this submission. We will review and provide feedback.

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, a couple of comments to help get this across the finish line

src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs Outdated Show resolved Hide resolved
src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs Outdated Show resolved Hide resolved
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes, another couple of things I missed the first time.

src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs Outdated Show resolved Hide resolved
src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs Outdated Show resolved Hide resolved
src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs Outdated Show resolved Hide resolved
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes and for your patience (I was out for a while).
@MaggieKimani1 for final review and merge

@baywet baywet merged commit c080552 into microsoft:vnext Jun 13, 2023
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.

4 participants