-
Notifications
You must be signed in to change notification settings - Fork 4k
Move ARM cmdlets from Resources to Resources.Rest proj #2125
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
@azuresdkci test this please |
@azuresdkci retest this please |
…MoveRG # Conflicts: # src/ResourceManager/Resources/Commands.ResourceManager/Cmdlets/SdkModels/Deployments/TemplateValidationInfo.cs # src/ResourceManager/Resources/Commands.ResourceManager/Cmdlets/SdkModels/Errors/PSResourceManagerError.cs # src/ResourceManager/Resources/Commands.ResourceManager/Cmdlets/SdkModels/ResourceProvider/PSResourceProviderLocationInfo.cs # src/ResourceManager/Resources/Commands.Resources.Test/Commands.Resources.Test.csproj # src/ResourceManager/Resources/Commands.Resources.Test/Models.ResourceGroups/ResourceClientTests.cs # src/ResourceManager/Resources/Commands.Resources.Test/packages.config # src/ResourceManager/Resources/Commands.Resources/Commands.Resources.csproj # src/ResourceManager/Resources/Commands.Resources/Models.ResourceGroups/PSResourceProviderLocation.cs # src/ResourceManager/Resources/Commands.Resources/Models.ResourceGroups/PSResourceProviderLocationInfo.cs # src/ResourceManager/Resources/Commands.Resources/Models.ResourceGroups/ResourceClient.ResourceManager.cs # src/ResourceManager/Resources/Commands.Resources/Models.ResourceGroups/ResourceClient.cs # src/ResourceManager/Resources/Commands.Resources/Models.ResourceGroups/ResourcesExtensions.cs # src/ResourceManager/Tags/Commands.Tags/Commands.Tags.csproj # src/ResourceManager/Tags/Commands.Tags/packages.config
…s test Fix RM test controller and build issues Merge branch 'dev' of https://github.com/Azure/azure-powershell into MoveRG
files in src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\Implementation\ResourceGroups have very long lines which makes it hard to view on github, please limit them to 100~120 symbols per line. |
in src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\Implementation\ResourceGroups\GetAzureResourceGroupCmdlet.cs, line 79. Instead of catching all the exceptions and silently continuing please write logs at debug stream - aka WriteDebug |
src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\SdkClient\ResourceManagerSdkClient.cs -> add license header. |
the naming convention for PS wrappers of model class is PS[nameOfTheModelToWrap]. you have several classes that violates that. Please fix:
|
src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\Utilities\TemplateUtility.cs - missing license header. |
src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\Utilities\TemplateUtility.cs, line 108. Please add exception details through WriteDebug. |
src\ResourceManager\Resources\Commands.ResourceManager\Cmdlets\Utilities\TemplateUtility.cs, line 172. Change Debug.Assert to argument exception |
due to huge changes please bump up major version in AzureRM.Resources.psd1 |
move strings to resources:
|
<value>Are you sure you want to cancel the active deployment of resource group '{0}'?</value> | ||
</data> | ||
<data name="CancelResourceGroupDeploymentMessage" xml:space="preserve"> | ||
<value>Cancelling active resource group deployment ...</value> |
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.
nit: no space after "deployment" and "..." or get line 121 in sync with this line
src\ResourceManager\Resources\Commands.Resources.Test\ScenarioTests\ResourcesController.cs, you should introduce an overload for RunPsTest which takes XunitTracingInterceptor as first parameter and then assigns it to helper.TracingInterceptor and should use that method from tests. (e.g. https://github.com/Azure/azure-powershell/blob/dev/src/ResourceManager/Profile/Commands.Profile.Test/ProfileController.cs#L57). This will let you have output logs from PS code execution. |
…named objects to fit PS{x}
@hovsepm I've addressed all the feedback, except for the last one: Adding the XunitTracingInterceptor to our test controller. I'm not comfortable making this change yet, as I don't know what the impact of this change will be on other RP projects that depend on this. If the above changes are merged, I will then be comfortable working on this tomorrow. Let me know if that sounds good. |
@hovsepm latest on demand job here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/847/ |
…MoveRG Conflicts: ChangeLog.md
@hovsepm latest sign job here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-sign/439/ |
Latest on demand here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/850/ |
Updated on demand here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/853/ |
@vivsriaus "This branch has conflicts that must be resolved" please pull from upstream and resolve conflicts |
…MoveRG Conflicts: src/ResourceManager/Resources/Commands.Resources.Test/Commands.Resources.Test.csproj
@hovsepm done. Latest on demand here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/855/ |
@hovsepm things looking good on this pr? |
@vivsriaus reviewing. |
This is a massive change list that impacts the core ARM cmdlets like ResourceGroups, deployments, features and providers. We're moving these cmdlets which are currently using the old HYAK client into the Commands.Rest project, using the new autorest SDK client. More updates will follow shortly