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 when rendering version enum only include current member and mark it modelAsString: true #923

Merged
merged 3 commits into from
May 25, 2024

Conversation

timotheeguerin
Copy link
Member

This ensure that when the version enum is still referenced that

  1. we still mark it as modelAsString true to not be flagged as a breaking change in spec repo(even though it won't matter as it gets special treatment)
  2. only include the emitted version member so adding new versions in the future don't update the previous one

@azure-sdk
Copy link
Collaborator

azure-sdk commented May 25, 2024

All changed packages have been documented.

  • @azure-tools/typespec-autorest
Show changes

@azure-tools/typespec-autorest - fix ✏️

When emitting version enum only include current version and mark with modelAsString: true

@azure-sdk
Copy link
Collaborator

@allenjzhang
Copy link
Member

allenjzhang commented May 25, 2024

#1 is good
#2 Would be better emit all versions up to current one instead a single current version? I guess as previous version like -preview could be taken out, it would then impact the swaggers. Depending on the client codegen and breaking change or not, you effectively can only do single version client instead of offer version selectors providing no breaking changes.

@allenjzhang
Copy link
Member

#1 is good
#2 Would be better emit all versions up to current one instead a single current version? I guess as previous version like -preview could be taken out, it would then impact the swaggers. Depending on the client codegen and breaking change or not, you effectively can only do single version client instead of offer version selectors providing no breaking changes.

Further thinking about it, swaggers/openapi, it only represent one version in our multi-version to single-version emission, it is accurate to render single version there. So I am good with this change.

@timotheeguerin
Copy link
Member Author

So this only applies if you reference the version enum in other cases we already omitted it.

In those case the only real case is when it's used in the paramerrized host.

I don't think it matters at all what that value is as it's not used in swagger generators but only selecting the current version does feel right and the simplest so we don't create extra diff for new versions

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