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

OpenAPI3: emit all properties for unreferenced schemas #2620

Merged
merged 6 commits into from
Nov 30, 2023

Conversation

tjprescott
Copy link
Member

@tjprescott tjprescott commented Nov 1, 2023

Fix #2571.

BREAKING CHANGE: Since the previous behavior was to emit unreferenced schemas with Read visibility, this change can produce a breaking change in Swagger if unreferenced schemas were previously emitted that had write visibility properties stripped.

Copy link
Contributor

github-actions bot commented Nov 1, 2023

Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status):
Playground: https://cadlplayground.z22.web.core.windows.net/prs/2620/

Website: https://tspwebsitepr.z22.web.core.windows.net/prs/2620/

@tjprescott tjprescott added the breaking-change A change that might cause specs or code to break label Nov 1, 2023
@tjprescott tjprescott marked this pull request as draft November 16, 2023 16:59
@tjprescott tjprescott force-pushed the unreferencedVisibilityFix branch 3 times, most recently from 3ac7a5c to fed834d Compare November 16, 2023 17:28
@tjprescott tjprescott marked this pull request as ready for review November 16, 2023 17:29
@tjprescott tjprescott force-pushed the unreferencedVisibilityFix branch 4 times, most recently from bfab436 to 35e58c8 Compare November 16, 2023 19:42
@tjprescott
Copy link
Member Author

tjprescott commented Nov 16, 2023

Investigating the weird cause of the "DeleteOrCreateOrUpdateOrReadItem" suffixes. It seems to be a very edgy edge case...

Playground

@tjprescott tjprescott force-pushed the unreferencedVisibilityFix branch 4 times, most recently from 85b0f1d to 52310e8 Compare November 21, 2023 20:49
@tjprescott tjprescott force-pushed the unreferencedVisibilityFix branch 2 times, most recently from 24fc7f7 to f251fe0 Compare November 29, 2023 18:26
@@ -112,7 +114,7 @@ components:
relatives:
type: array
items:
$ref: '#/components/schemas/PersonRelative'
$ref: '#/components/schemas/PersonRelativeReadOrCreateOrUpdateOrDeleteItem'
Copy link
Member Author

Choose a reason for hiding this comment

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

These are, per our discussion, the appropriate names.

packages/openapi3/src/openapi.ts Outdated Show resolved Hide resolved
packages/openapi3/src/visibility-usage.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

Travis, this looks great, can we try this out in the rest-api-specs repo to see how many specs are affected?

@tjprescott
Copy link
Member Author

@markcowl here's the link to the REST API specs test PR:
Azure/azure-rest-api-specs#26901

tjprescott added a commit to Azure/azure-rest-api-specs that referenced this pull request Nov 30, 2023
@tjprescott
Copy link
Member Author

@markcowl the REST API specs pass with this change:
image

@tjprescott tjprescott enabled auto-merge (squash) November 30, 2023 18:00
@tjprescott tjprescott merged commit 52d0a8b into microsoft:main Nov 30, 2023
10 checks passed
@tjprescott tjprescott deleted the unreferencedVisibilityFix branch November 30, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change A change that might cause specs or code to break
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unreachable types are resolved with read visibility
3 participants