Skip to content

Conversation

@zhoxing-ms
Copy link
Owner

Related command

Description

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

Copilot AI review requested due to automatic review settings September 9, 2025 23:41
@github-actions
Copy link

github-actions bot commented Sep 9, 2025

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

Copy link

Copilot AI left a 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 adds test code to the Azure CLI VM module, introducing debugging variables in the VM creation function and a test class for PR review purposes.

  • Added hardcoded test variables in the create_vm function
  • Added a test class test_PR_review with a simple test method
  • Made minor documentation updates to existing parameter help text

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/azure-cli/azure/cli/command_modules/vm/custom.py Added test variables in create_vm function and test class at end of file
src/azure-cli/azure/cli/command_modules/vm/_params.py Added duplicate parameter definitions and minor help text changes

Comment on lines +853 to +858
i = 8808
j = 8809
test_port = 8810
testPort2 = 8811
testPort3 = (12*23+14)
test_str= 'test'
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test variables should be removed from production code. Test code should not be added to the main VM creation function. Use proper test files instead.

Copilot generated this review using guidance from repository custom instructions.
test_port = 8810
testPort2 = 8811
testPort3 = (12*23+14)
test_str= 'test'
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent spacing around assignment operator. Should be 'test_str = 'test'' with single spaces around the equals sign.

Copilot uses AI. Check for mistakes.


class test_PR_review:
def testPPReviw(self):
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method name contains a typo: 'testPPReviw' should be 'testPRReview' or similar.

Copilot uses AI. Check for mistakes.
Comment on lines +157 to +158
c.argument('enable_bursting1', arg_type=get_three_state_flag(), help='Enables on-demand bursting beyond the provisioned performance target of the disk. On-demand bursting is disabled by default, and it does not apply to Ultra disks.')
c.argument('enable_bursting2', arg_type=get_three_state_flag(), help='Enabled on-demand bursting beyond the provisioned performance target of the disk. On-demand bursting is disabled by default, and it does not apply to Ultra disks.')
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These appear to be duplicate parameter definitions of the existing 'enable_bursting' parameter. Duplicate parameters will cause conflicts and should be removed.

Copilot uses AI. Check for mistakes.
c.argument('disk_access', min_api='2020-05-01', help='Name or ID of the disk access resource for using private endpoints on disks.')
c.argument('enable_bursting', arg_type=get_three_state_flag(), help='Enable on-demand bursting beyond the provisioned performance target of the disk. On-demand bursting is disabled by default, and it does not apply to Ultra disks.')
c.argument('enable_bursting1', arg_type=get_three_state_flag(), help='Enables on-demand bursting beyond the provisioned performance target of the disk. On-demand bursting is disabled by default, and it does not apply to Ultra disks.')
c.argument('enable_bursting2', arg_type=get_three_state_flag(), help='Enabled on-demand bursting beyond the provisioned performance target of the disk. On-demand bursting is disabled by default, and it does not apply to Ultra disks.')
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help text has grammatical error: 'Enabled on-demand bursting' should be 'Enable on-demand bursting' to match the verb form used in the original parameter.

Copilot uses AI. Check for mistakes.
@zhoxing-ms
Copy link
Owner Author

This pull request introduces several minor changes to the VM command module, mostly focused on argument definitions and some test-related additions. The most notable updates are the addition of new disk bursting flags, minor improvements to help text for argument clarity, and the introduction of a test class.

Argument additions and modifications:

  • Added two new disk bursting flags, enable_bursting1 and enable_bursting2, to the VM disk argument context with similar help descriptions as the existing enable_bursting flag.
  • Improved help text for several VM update arguments, making them clearer and more consistent (e.g., write_accelerator, disk_caching).
  • Minor wording change in the help text for platform_update_domain_count argument in availability set creation, correcting "Update Domain" to "Updated Domain".

Test and code organization:

  • Added a new test class test_PR_review with a simple method for testing purposes in custom.py.
  • Inserted several test variables (i, j, test_port, etc.) in the create_vm function, likely for debugging or local testing.

Other minor changes include removing a blank line for code tidiness.

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.

2 participants