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

Bring NetApp Files to R3.5 standard #5565

Closed
wants to merge 9 commits into from

Conversation

leonardbf
Copy link
Contributor

@leonardbf leonardbf commented Mar 24, 2019

Includes export policy and active directory.
Still in preview - breaking changes are not an issue

Swagger:
https://github.com/leonardbf/azure-rest-api-specs/tree/master/specification/netapp/resource-manager

Last swagger PR:
Azure/azure-rest-api-specs#5446

@leonardbf
Copy link
Contributor Author

@dsgouda you have added the changes requested label but I don't see any actions stated. Is there something I'm required to do? Thanks.

@dsgouda
Copy link
Contributor

dsgouda commented Mar 27, 2019

@leonardbf Please pull down latest changes from master and resolve the conflicts or recreate the changes branch and open a new PR

@leonardbf
Copy link
Contributor Author

@dsgouda I have pulled from upstream into my branch.

@dsgouda dsgouda changed the base branch from psSdkJson6 to master March 29, 2019 16:36
@dsgouda
Copy link
Contributor

dsgouda commented Mar 29, 2019

@leonardbf I missed that you were targeting the psSdkJson6 branch, we have switched our default branch from psSdkJson6 to master. Please update the branch again with master, I shall post my review in the meantime.

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Left a few comments

@@ -26,7 +26,7 @@
</PropertyGroup>

<PropertyGroup>
<TargetFrameworks>$(SdkTargetFx)</TargetFrameworks>
<TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I shall change it back.

@@ -8,7 +8,7 @@
<Description>Provides NetApp storage management capabilities for Microsoft Azure.</Description>
<AssemblyTitle>Microsoft Azure NetApp Management</AssemblyTitle>
<AssemblyName>Microsoft.Azure.Management.NetApp</AssemblyName>
<Version>0.9.0-preview</Version>
<Version>0.9.1-preview</Version>
<PackageTags>MicrosoftAzure Management;NetApp</PackageTags>
<PackageReleaseNotes>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the PackageReleaseNotes

@@ -4,7 +4,7 @@
<PackageId>NetApp.Tests</PackageId>
<Description>NetApp.Tests Class Library</Description>
<AssemblyName>NetApp.Tests</AssemblyName>
<Version>1.0.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.

Test projects are never published. Please set this to 1.0.0 and do not modify henceforth

@leonardbf
Copy link
Contributor Author

@dsgouda Yes I was unaware the target should now be master. I have pulled latest from upstream master, merged my changes there and created a new PR #5646. It includes changes for your comments above. I will close this PR when you have responded concerning the new one.

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.

3 participants