Skip to content
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

Creating a separate module for StackHCIVM #23414

Merged
merged 80 commits into from
Nov 28, 2023

Conversation

hvedati
Copy link

@hvedati hvedati commented Nov 14, 2023

Description

Checklist

  • SHOULD select appropriate branch. Cmdlets from Autorest.PowerShell should go to generation branch.
  • SHOULD make the title of PR clear and informative, and in the present imperative tense.
  • SHOULD update ChangeLog.md file(s) appropriately
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense. Add changelog in description section if PR goes into generation branch.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD have approved design review for the changes in this repository (Microsoft internal only) with following situations
    • Create new module from scratch
    • Create new resource types which are not easy to conform to Azure PowerShell Design Guidelines
    • Create new resource type which name doesn't use module name as prefix
    • Have design question before implementation
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT introduce breaking changes in Az minor release except preview version.
  • SHOULD NOT adjust version of module manually in pull request

@NoriZC
Copy link
Contributor

NoriZC commented Nov 21, 2023

Hi @hvedati,
As you are separating StackHCIVM from StackHCI, we will think this module as a new one.

For new modules onboard, we recommend using autorest powershell v4 for code generation. But in your module, you are still using the namespace which comes from the v3 generated module. For current case, please update the namespaces in all custom files: Microsoft.Azure.PowerShell.Cmdlets.StackHCIVM.Models.Api20230901Preview.IVirtualMachineInstance => Microsoft.Azure.PowerShell.Cmdlets.StackHCIVM.Models.IVirtualMachineInstance

The reason why tou are not reproducing the error locally is you are not using the newest version of autorest locally.

Please do following steps:

  • Run autorest --reset to make the default version as v4
  • Update all the custom files to fit the autorest powershell v4 generated namespaces.
  • Please also change the file names from StackHCIVm to StackHCIVM

@NoriZC NoriZC force-pushed the dev/hv/addingStackHCIVmPreviewModule branch from e66aa7a to 3dfd070 Compare November 22, 2023 07:22
@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@NoriZC NoriZC self-assigned this Nov 23, 2023
@NoriZC NoriZC changed the title Creating a separate module for StackHCIVm Creating a separate module for StackHCIVM Nov 23, 2023
NoriZC
NoriZC previously approved these changes Nov 23, 2023
@NoriZC NoriZC requested a review from isra-fel November 23, 2023 11:34
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

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

Hi @hvedati

  1. Turned out there are a few misspellings left. Please check the screenshot.
  2. Custom cmdlets need to include some common parameters to support testing and proxy, etc. Please follow this guide to add them.
  3. After adding the common parameters, the custom cmdlets can be recorded normally. Please make sure to add test cases.

image

Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@hvedati hvedati force-pushed the dev/hv/addingStackHCIVmPreviewModule branch from dc0ab80 to 6aaf2d7 Compare November 27, 2023 04:27
@NoriZC NoriZC dismissed isra-fel’s stale review November 28, 2023 01:40

Comments addressed

@NoriZC NoriZC merged commit d659008 into Azure:generation Nov 28, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants