-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Remove obsolete Invoke-TypeSpecDataPlaneGenerateSDKPackage.ps1 script and modernize template for TypeSpec #53543
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
base: main
Are you sure you want to change the base?
Conversation
… and update documentation - Deleted eng/scripts/automation/Invoke-TypeSpecDataPlaneGenerateSDKPackage.ps1 as it is not using the new emitter path - Updated doc/DataPlaneCodeGeneration/AzureSDKPackage_Setup.md to recommend using tsp-client init or manually creating tsp-location.yaml - Added guidance on required emitterPackageJsonPath property for new services Co-authored-by: JoshLove-msft <[email protected]>
| We will use the Azure SDK template [Azure.Template](https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/template/Azure.Template) to create the initial project skeleton for SDKs generated from TypeSpec. | ||
|
|
||
| You can run `eng\scripts\automation\Invoke-TypeSpecDataPlaneGenerateSDKPackage.ps1` to generate the starting SDK client library package directly as following: | ||
| ### Option 1: Using tsp-client init (Recommended) |
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.
@copilot let's just delete this file. We already have a separate doc that provides the same info.
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.
Done. Deleted AzureSDKPackage_Setup.md and updated the reference in AzureSDKGeneration_Prerequistites.md to point to AzureSDKCodeGeneration_DataPlane_Quickstart.md instead (commit 869fba0).
- Deleted doc/DataPlaneCodeGeneration/AzureSDKPackage_Setup.md as requested - content is covered in AzureSDKCodeGeneration_DataPlane_Quickstart.md - Updated reference in AzureSDKGeneration_Prerequistites.md to point to the quickstart guide instead Co-authored-by: JoshLove-msft <[email protected]>
| ## Set up SDK Project Folder | ||
|
|
||
| You will need to generate the SDK library under an SDK project folder following the repository [guidelines](https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/AzureSDKPackage_Setup.md). | ||
| You will need to generate the SDK library under an SDK project folder following the repository [guidelines](https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/AzureSDKCodeGeneration_DataPlane_Quickstart.md). |
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.
@copilot we should just remove this link as it is linking back to the same document where this md is hosted.
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.
Done. Removed the circular link from the prerequisites doc (commit 4415299).
- Removed link from AzureSDKGeneration_Prerequistites.md that was linking back to AzureSDKCodeGeneration_DataPlane_Quickstart.md - The quickstart already references the prerequisites, so this created a circular reference Co-authored-by: JoshLove-msft <[email protected]>
| ## Set up SDK Project Folder | ||
|
|
||
| You will need to generate the SDK library under an SDK project folder following the repository [guidelines](https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/AzureSDKPackage_Setup.md). | ||
| You will need to generate the SDK library under an SDK project folder following the repository guidelines. |
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.
@copilot lets link to the template as an example. https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/template/Azure.Template.
Let's also update the template to remove autorest.md and instead add a tsp-location.yaml.
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.
Done. Updated prerequisites doc to link to Azure.Template as an example. Also updated the template itself: removed autorest.md and added tsp-location.yaml with placeholder values (commit 088d0d6).
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.
@copilot let's also remove <IncludeAutorestDependency>true</IncludeAutorestDependency> from the csproj
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.
The comment appears incomplete. Should I remove <IncludeAutorestDependency>true</IncludeAutorestDependency> from the Azure.Template.csproj since we replaced autorest.md with tsp-location.yaml?
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.
@copilot yes
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.
Done. Removed IncludeAutorestDependency from Azure.Template.csproj (commit 136ee56).
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.
@copilot let's also delete the local swagger folder in this project.
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.
Done. Deleted the swagger folder from Azure.Template (commit bcf2541).
- Added link to Azure.Template in AzureSDKGeneration_Prerequistites.md as an example - Removed autorest.md from Azure.Template (obsolete for TypeSpec) - Added tsp-location.yaml to Azure.Template with placeholder values as a template for new services Co-authored-by: JoshLove-msft <[email protected]>
- Removed IncludeAutorestDependency property since template now uses TypeSpec with tsp-location.yaml instead of autorest.md Co-authored-by: JoshLove-msft <[email protected]>
- Removed sdk/template/Azure.Template/src/swagger folder as it's no longer needed for TypeSpec-based generation Co-authored-by: JoshLove-msft <[email protected]>
| # This is a template tsp-location.yaml file for TypeSpec-based SDK generation | ||
| # Replace the placeholder values with your actual TypeSpec project information | ||
|
|
||
| directory: specification/<service-name>/<typespec-project-folder> |
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.
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.
Done. Updated the directory to point to specification/widget/data-plane/WidgetAnalytics (commit de01e0c).
- Updated directory to point to specification/widget/data-plane/WidgetAnalytics as a concrete example instead of placeholder values Co-authored-by: JoshLove-msft <[email protected]>
| # Replace the placeholder values with your actual TypeSpec project information | ||
|
|
||
| directory: specification/widget/data-plane/WidgetAnalytics | ||
| commit: <commit-hash-from-azure-rest-api-specs> |
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.
Update to new commit once #53550 is resolved
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.
@copilot update to use the spec from here Azure/azure-rest-api-specs#38490
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.
Added reference to PR 38490 with a TODO comment to update the commit hash once the PR is merged (commit 8b7371e).
- Added TODO comment to update commit once PR 38490 is merged - Updated placeholder commit reference to indicate it should come from PR 38490 Co-authored-by: JoshLove-msft <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Plan to address issue: Invoke-TypeSepcDataPlaneGenerateSDKPackage.ps1 is not using new emitter path
Based on the issue discussion and team decision:
eng/scripts/automation/Invoke-TypeSpecDataPlaneGenerateSDKPackage.ps1doc/DataPlaneCodeGeneration/AzureSDKPackage_Setup.mdto recommend usingtsp-client initor manual creation oftsp-location.yamlAzureSDKPackage_Setup.mdfile per PR feedbackAzureSDKGeneration_Prerequistites.mdOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.