Skip to content

Improve message for error "incompatible properties" #316

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.

Sign up for GitHub

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

Jump to bottom
Merged
mikeharder merged 11 commits into from
Apr 3, 2024
Merged

Improve message for error "incompatible properties" #316

mikeharder merged 11 commits into from
Apr 3, 2024

Conversation

@mikeharder
Copy link
Member

@mikeharder mikeharder commented Mar 29, 2024 •
edited
Loading

Example where previous error was insufficient to help spec author: Azure/azure-sdk-tools#7917

Old

incompatible properties : bar

New

incompatible properties : bar
  definitions/FooBarString/properties/bar
    at file:///home/user/openapi-diffsrc/test/specs/incompatible-properties.json#L13:8
  definitions/FooBarObject/properties/bar
    at file:///home/user/openapi-diff/src/test/specs/incompatible-properties.json#L26:8

@mikeharder mikeharder marked this pull request as ready for review April 3, 2024 00:13
mikeharder
mikeharder commented Apr 3, 2024
View reviewed changes
src/lib/validators/openApiDiff.ts
throw new Error(stderr)
}
const resolveSwagger = new ResolveSwagger(outputFilePath)

Copy link
Member Author

@mikeharder mikeharder Apr 3, 2024

Choose a reason for hiding this comment

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

Change is just moving this code earlier, and passing map to new ResolveSwagger() so it can include original locations in errors.

timotheeguerin
timotheeguerin reviewed Apr 3, 2024
View reviewed changes
@mikeharder mikeharder self-assigned this Apr 3, 2024
@mikeharder mikeharder mentioned this pull request Apr 3, 2024
Closed
@mikeharder
Copy link
Member Author

mikeharder commented Apr 3, 2024

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Apr 3, 2024

Azure Pipelines successfully started running 1 pipeline(s).

konrad-jamrozik
konrad-jamrozik reviewed Apr 3, 2024
View reviewed changes
konrad-jamrozik
konrad-jamrozik reviewed Apr 3, 2024
View reviewed changes
konrad-jamrozik
konrad-jamrozik approved these changes Apr 3, 2024
View reviewed changes
Copy link

@konrad-jamrozik konrad-jamrozik left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left some suggestions on how to make the tests easier to comprehend. Besides, LGTM!

@mikeharder mikeharder enabled auto-merge (squash) April 3, 2024 06:10
@mikeharder mikeharder merged commit 6159d96 into Azure:main Apr 3, 2024
@mikeharder mikeharder deleted the incompatible-properties branch April 3, 2024 06:24
@mikeharder mikeharder mentioned this pull request Apr 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timotheeguerin timotheeguerin timotheeguerin approved these changes

@markcowl markcowl Awaiting requested review from markcowl

+1 more reviewer

@konrad-jamrozik konrad-jamrozik konrad-jamrozik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mikeharder mikeharder

Labels

None yet

Projects

Azure SDK EngSys ❄️🎄🎁🎅✨
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve message for error "incompatible properties"

3 participants

@mikeharder @timotheeguerin @konrad-jamrozik