-
Notifications
You must be signed in to change notification settings - Fork 229
Improve VerifySetup Output Clarity #13100
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
Conversation
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
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.
Pull request overview
This PR improves the clarity of the VerifySetup tool's output by adding support for requirement explanations and enhancing installation instructions. The changes enable Copilot to provide better context about why specific requirements exist and tailor installation guidance to the user's shell environment.
- Adds a
reasonfield to explain why certain requirements are needed (e.g., why Python is a core requirement) - Updates Python tool installation instructions to specifically reference the Python SDK repository directory
- Enhances Copilot instructions to organize output into categorical sections and provide shell-specific commands
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/Verify/VerifySetupTool.cs | Adds Reason field population when requirement checks fail with messages |
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/SetupRequirements.cs | Defines nullable reason property in the Requirement model |
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/VerifySetupResponse.cs | Adds nullable Reason property to RequirementCheckResult response model |
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Configuration/RequirementsV1.json | Adds reason explanations for Python and azpysdk requirements; clarifies Python tool installation instructions to specify Python SDK directory |
| eng/common/instructions/azsdk-tools/verify-setup.instructions.md | Updates output instructions to explain reasons, organize by categories, and provide shell-specific commands |
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/Verify/VerifySetupTool.cs
Outdated
Show resolved
Hide resolved
praveenkuttappan
left a comment
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.
Looks good. One nit suggestion from copilot about ?? operator is good to take.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#13100 See [eng/common workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow) --------- Co-authored-by: jennypng <63012604+JennyPng@users.noreply.github.com>
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#13100 See [eng/common workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow) --------- Co-authored-by: jennypng <63012604+JennyPng@users.noreply.github.com>
* prompt improvement * minor * test making copilot install for you * add reason field * minor * remove unnecessary null operator Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
closes #13001
closes #13240
related to point 2 in #13248 (python installs should be specifically from python SDK directory)
reasonto tool response for checks that are confusing, e.g. why is Python a core requirement?Only checking core requirements in tools repo with gpt-4o in WSL (note that reason is explained for Python):

checking go reqs with claude sonnet 4

checking python reqs w claude sonnet 4.5 (note that reason for azpysdk is given):
