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.

Sign up for GitHub

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

Jump to bottom

Updated Management.ResourceManager sdk models and tests for new api version. #7419

Merged
merged 11 commits into from
Sep 6, 2019
Merged

Updated Management.ResourceManager sdk models and tests for new api version. #7419

merged 11 commits into from
Sep 6, 2019

Conversation

sandipsh
Copy link
Contributor

Updated policy tests for enforcementMode and data plane modes.

@sandipsh sandipsh requested a review from erich-wang as a code owner August 28, 2019 22:03
@pilor
Copy link
Contributor

pilor commented Aug 28, 2019 •
edited
Loading

"Entries": [

looks like this is not in the right sessionRecords folder #Closed

Refers to: sdk/resources/Microsoft.Azure.Management.Resource/tests/SessionRecords/Policy.Tests.LivePolicyTests/CanCrudDataPlanePolicyDefinition.json:2 in abfbd6f. [](commit_id = abfbd6f, deletion_comment = False)

@isra-fel isra-fel added Mgmt This issue is related to a management-plane library. needs-review labels Aug 29, 2019
@isra-fel isra-fel self-assigned this Aug 29, 2019
@pilor
Copy link
Contributor

pilor commented Aug 29, 2019

I believe the csproj and maybe AssemblyInfo need to have their versions updated (may be wrong, been a long time)

@pilor
Copy link
Contributor

pilor commented Aug 29, 2019 •
edited
Loading

I believe the csproj and maybe AssemblyInfo need to have their versions updated (may be wrong, been a long time)

.props as well according to this: https://github.com/Azure/azure-sdk-for-net/blob/master/Documentation/code-generation-instructions.md

@pilor
Copy link
Contributor

pilor commented Aug 29, 2019

LGTM. Please get an ack from Tian Ouyang and Gaurav Bhatnagar that you are pushing a new Management.Resource SDK version before completing this PR.

isra-fel
isra-fel suggested changes Aug 29, 2019
View reviewed changes
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.

@sandipsh ,

You are not only updating the tests, are you? It would be great if the title correctly reflects the changes.
Anyway, when you are generating code, please also track the eng\mgmt\mgmtmetadata\<rpName>.txt file, which gets updated when you run generate.ps1.
And could you also put a link to the swagger review PR in the description?
Otherwise it looks good to me. Thanks

@isra-fel isra-fel assigned sandipsh and unassigned isra-fel Aug 29, 2019
@sandipsh sandipsh changed the title Updated tests for new api version. Updated Management.ResourceManager sdk models and tests for new api version. Aug 30, 2019
@sandipsh
Copy link
Contributor Author

swagger PR is here - Azure/azure-rest-api-specs#6998

@sandipsh sandipsh changed the title Updated Management.ResourceManager sdk models and tests for new api version. Updated Management.ResourceManager sdk models and tests for new api version. swagger PR - https://github.com/Azure/azure-rest-api-specs/pull/6998 Aug 30, 2019
@sandipsh sandipsh changed the title Updated Management.ResourceManager sdk models and tests for new api version. swagger PR - https://github.com/Azure/azure-rest-api-specs/pull/6998 Updated Management.ResourceManager sdk models and tests for new api version. Aug 30, 2019
@isra-fel
Copy link
Member

isra-fel commented Sep 2, 2019 •
edited
Loading

@sandipsh Thanks for the update, though the eng\mgmt\mgmtmetadata\<rpName>.txt file still needs to be added to the PR. See https://github.com/Azure/adx-documentation-pr/blob/master/engineering/adx_netsdk_process.md#sdk-generation-from-updated-spec

Tiano2017
Tiano2017 reviewed Sep 3, 2019
View reviewed changes
...es/Microsoft.Azure.Management.Resource/src/Microsoft.Azure.Management.ResourceManager.csproj Outdated
@@ -7,7 +7,7 @@
<PackageId>Microsoft.Azure.Management.ResourceManager</PackageId>
<Description>Provides resource group and resource management capabilities for Microsoft Azure.</Description>
<AssemblyName>Microsoft.Azure.Management.ResourceManager</AssemblyName>
<Version>2.4.0-preview</Version>
<Version>2.5.0-preview</Version>
Copy link
Contributor

Choose a reason for hiding this comment

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

2.5.0-preview [](start = 11, length = 13)

I don't think 2.4.0-preview was ever released. you may not need a new version.

Tiano2017
Tiano2017 reviewed Sep 5, 2019
View reviewed changes
sdk/resources/Microsoft.Azure.Management.Resource/src/generate.ps1 Outdated
Start-AutoRestCodeGeneration -ResourceProvider "resources/resource-manager" -SdkRepoRootPath "$PSScriptRoot\..\..\..\.." -AutoRestVersion "latest" -AutoRestCodeGenerationFlags "--tag=package-policy-2019-06" -SdkGenerationDirectory "$PSScriptRoot\Generated"
Copy link
Contributor

Choose a reason for hiding this comment

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

--tag=package-policy-2019-06" [](start = 177, length = 29)

instead of updating the cmd, can we add a new line of command and leave it as commented?

@Tiano2017
Copy link
Contributor

Looks good to me.

@isra-fel isra-fel merged commit 554d96c into Azure:master Sep 6, 2019
JoshLove-msft pushed a commit to JoshLove-msft/azure-sdk-for-net that referenced this pull request Sep 10, 2019
@sandipsh @JoshLove-msft
Updated Management.ResourceManager sdk models and tests for new api v…
4eeca58 
…ersion. (Azure#7419)

* updated tests for new api version.

* Fixed merging of couple of models and a record file.

* minor: adjusted whitepacing

* Fixed test.

* updated .csproj, .props and assembly info.

* reverting proj version to 2.4.0-preview as it is the latest.

* updated generate.ps1 and resources_resource-manager.txt tracking file.

* updated generate.ps1 as suggested.

* updated generate.ps1 as suggested.

* correct the line that needs to be commented out
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tiano2017 Tiano2017 Tiano2017 left review comments

@RamyasreeChakka RamyasreeChakka RamyasreeChakka approved these changes

@isra-fel isra-fel isra-fel approved these changes

@pilor pilor pilor approved these changes

@erich-wang erich-wang Awaiting requested review from erich-wang

Assignees

@sandipsh sandipsh

Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sandipsh @pilor @isra-fel @Tiano2017 @RamyasreeChakka