-
Notifications
You must be signed in to change notification settings - Fork 75
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
Known* definitions mission in TS Generation #2476
Comments
@sarangan12 will the event grid generated as the modular SDK? is this SDK GAed? This is the intermedia state for extensible enum and there are two design issues in progress. And we need to support it once the designed issue is finalized.
/cc @joheredi for awareness |
3 tasks
sarangan12
added a commit
to Azure/azure-sdk-for-js
that referenced
this issue
Jun 7, 2024
### Packages impacted by this PR @azure/eventgrid-system-events ### Issues associated with this PR NA ### Describe the problem that is addressed by this PR 1. Originally, there was one unified package - [`@Azure/eventgrid`](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/eventgrid/eventgrid). This package was generated from Swagger specification. 2. Earlier in April 2024, some parts of the `@Azure/eventgrid` package were extracted out to create a new package - [`@azure/eventgrid-namespaces`](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/eventgrid/eventgrid-namespaces). This package was generated from Typespec specification. 3. Now, with this PR, we are looking at the second split of the `@Azure/eventgrid` package. All System Events have been extracted out of the swagger specification to Typespec specification. This is a new package named `@azure/eventgrid-system-events`. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? 1. In the Typespec core generator, We have an issue where client name overrides are not applied to `alias`. I have created an issue microsoft/typespec#3240 to fix this. In the meantime, I have handled this issue in the custom code. 2. With the new generator, `Known*` models are not generated. I have created an issue Azure/autorest.typescript#2476 to fix this. In the meantime, I have handled this issue in the custom code. 3. In the previous generator, we had several properties which were overridden as mandatory parameters. With the new generator, such mass overrides are not possible. So, we are allowing them to be optional and close to the Wore specification. This is in parity with the other language SDKs. ### Are there test cases added in this PR? _(If not, why?)_ No. They are all model objects. No code functionality is involved. ### Provide a list of related PRs _(if any)_ 1. microsoft/typespec#3240 2. Azure/autorest.typescript#2476 3. #29245 ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ `npx tsp compile client.tsp` ### Checklists - [X] Added impacted package name to the issue description - [X] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [X] Added a changelog (if necessary) @xirzec @joheredi @lmazuel FYI...... --------- Co-authored-by: Harsha Nalluru <sanallur@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For the following Typespec definition:
Now, If you look at the generated SDK, it looks like the following:
But, If you look at the similar swagger generated SDK, you can find a definition as:
i.e You should always have a Known* Model generated. But, it is not happening on the TS Generator. I am expecting a similar definition in the Typespec generated SDK also.
@xirzec @lmazuel FYI
The text was updated successfully, but these errors were encountered: