-
Notifications
You must be signed in to change notification settings - Fork 184
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
Improve SDK Type Inference in SDK Automation #9603
base: main
Are you sure you want to change the base?
Conversation
// NOTE: unbranded sdk will generate modular client bt default | ||
if (!isAzureSDK) return SDKType.ModularClient; | ||
|
||
const isManagementPlane = new RegExp(/\.Management[\/\\]?$/).test( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if we still need this, we already adopted default behavior(but not released yet) in codegen side: Azure/autorest.typescript#2973.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, we don't need to check it.
However, we have an independent changelog generation tool. GetSDKType()
will be used to bump version - need to find api-version type (stable or preview) based on different client type. I'm wondering if codegen side can add 'api-version' into package.json or somewhere else, then I don't need the check of 'management' any more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could directly refer the package name: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml#L39.
Starting with @azure
would be Modular and starting with @azure-rest
would be RLC.
I'm wondering if codegen side can add 'api-version' into package.json or somewhere else
I may not get your point, the api-version
is the package version or spec version? We have an issue to list all service apiVersion in README: Azure/autorest.typescript#2588.
Fix: #9520
Problem: Service team doesn't set
isModularClientLibrary
explicitly, we can improve user experience by inferencing sdk type automaticallySolution: add the Inference flow below