NetApp SDK initial version by leonardbf · Pull Request #5094 · Azure/azure-sdk-for-net

leonardbf · 2018-12-16T11:09:41Z

From new swagger spec Azure/azure-rest-api-specs#4891
(NetApp internal ticket NFSAAS-1504)

Description

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

Please add REST spec PR link to the SDK PR
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

…or-net into NFSAAS-1504-NetApp_SDK

shahabhijeet

@leonardbf you are releasing your first SDK + your swagger is still in preview.
Please make changes as suggested to publish your SDK in preview.
Please use the generate.ps1 to generate your SDK.
If you use generate.ps1, it creates the appropriate meta data that is required for the SDK.

shahabhijeet · 2018-12-19T00:16:22Z

src/SDKs/NetApp/Management.NetApp/Microsoft.Azure.Management.NetApp.csproj

+        <Description>Provides NetApp storage management capabilities for Microsoft Azure.</Description>
+        <AssemblyTitle>Microsoft Azure NetApp Management</AssemblyTitle>
+        <AssemblyName>Microsoft.Azure.Management.NetApp</AssemblyName>
+        <Version>1.0.0</Version>


@leonardbf if you are going as your v1, I recommend you go as preview and mark your version as 0.9.0-preview

ok, changed to 0.9.0-preview.

shahabhijeet · 2018-12-19T00:17:36Z

src/SDKs/NetApp/Management.NetApp/Microsoft.Azure.Management.NetApp.csproj

+        <PackageTags>MicrosoftAzure Management;NetApp</PackageTags>
+        <PackageReleaseNotes>
+            <![CDATA[
+                * Initial version


@leonardbf if this is your first version, please provide any links to your service or provide a description what this SDK will allow customers to do.
This is your first version that will let developers know what this service is about and what can be done with it.
If you have links to samples, provide that also.

ok, description added, including some examples. Note that a more formal description is pending anyway.

shahabhijeet · 2018-12-19T00:18:19Z

src/SDKs/NetApp/Management.NetApp/Microsoft.Azure.Management.NetApp.csproj

+    <PropertyGroup>
+        <TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
+    </PropertyGroup>
+    <ItemGroup Condition="'$(TargetFramework)' == 'net452'">


@leonardbf remove line 22 - 33

shahabhijeet · 2018-12-19T00:23:13Z

src/SDKs/NetApp/Management.NetApp/Generated/SdkInfo_MicrosoftNetApp.cs

+          {
+              return new Tuple<string, string, string>[]
+              {
+                new Tuple<string, string, string>("NetApp", "Accounts", "2017-08-15"),


@leonardbf why your API version is not tagged appropriately.
I see the swagger is inside a preview directory, but the API itself is marked as stable.
Please clarify if your service is preview or is it stable?

The API is being made public but at this time is still subject to more change than will be expected later. Therefore prefer to leave as preview for now.

dsgouda · 2018-12-19T18:56:31Z

src/SDKs/NetApp/Management.NetApp/Properties/AssemblyInfo.cs

+[assembly: AssemblyCulture("")]
+[assembly: NeutralResourcesLanguage("en")]
+
+[assembly: AssemblyVersion("1.0.0.0")]


Please update the version number here too

dsgouda · 2018-12-19T18:56:45Z

src/SDKs/NetApp/Management.NetApp/Microsoft.Azure.Management.NetApp.csproj

+    <!-- Please do not move/edit code below this line -->
+    <Import Condition=" Exists('$([MSBuild]::GetPathOfFileAbove(AzSdk.RP.props))') " Project="$([MSBuild]::GetPathOfFileAbove('AzSdk.RP.props'))" />
+    <!-- Please do not move/edit code above this line -->
+</Project>


nit: remove trailing lines

dsgouda · 2018-12-19T18:57:38Z

src/SDKs/NetApp/NetApp.Tests/NetApp.Tests.csproj

+  <PropertyGroup>
+    <PackageId>NetApp.Tests</PackageId>
+    <Description>NetApp.Tests Class Library</Description>
+    <VersionPrefix>1.0.0</VersionPrefix>


Replace VersionPrefix with Version

dsgouda · 2018-12-19T18:58:47Z

src/SDKs/NetApp/NetApp.Tests/NetApp.Tests.csproj

+    <Description>NetApp.Tests Class Library</Description>
+    <VersionPrefix>1.0.0</VersionPrefix>
+    <Authors>Microsoft Corporation</Authors>
+    <AssemblyName>NetApp.Tests</AssemblyName>


Please add <TreatWarningsAsErrors>true</TreatWarningsAsErrors> here and fix all errors

dsgouda · 2018-12-19T18:59:03Z

src/SDKs/NetApp/NetApp.Tests/ResourceTests/AccountTests.cs

+                    netAppMgmtClient.Accounts.CreateOrUpdate(netAppAccount, ResourceUtils.resourceGroup, ResourceUtils.accountName1);
+                    Assert.True(false);
+                }
+                catch (Exception ex)


Consider using Assert.Exception here

This was actually already considered but was unavailable. Leaving as is.

With review corrections.

dsgouda

LGTM

dsgouda

Looks good apart from a minor comment. Please address it in a later commit,

dsgouda · 2019-01-04T23:15:45Z

src/SDKs/NetApp/NetApp.Tests/NetApp.Tests.csproj

+    <PackageId>NetApp.Tests</PackageId>
+    <Description>NetApp.Tests Class Library</Description>
+    <AssemblyName>NetApp.Tests</AssemblyName>
+    <Version>1.0.0-preview</Version>


nit: Test projects must always be versioned as 1.0.0

leonardbf added 3 commits December 16, 2018 10:57

NFSAAS-1504 NetApp SDK initial version

8140c6f

NFSAAS-1504 NetApp SDK initial version

5ca40aa

Merge branch 'psSdkJson6' of https://github.com/leonardbf/azure-sdk-f…

5f547ea

…or-net into NFSAAS-1504-NetApp_SDK

shahabhijeet suggested changes Dec 19, 2018

View reviewed changes

shahabhijeet added the Changes Requested label Dec 19, 2018

dsgouda suggested changes Dec 19, 2018

View reviewed changes

NFSAAS-1504 NetApp SDK Initial Version

542c764

With review corrections.

dsgouda approved these changes Jan 4, 2019

View reviewed changes

shahabhijeet approved these changes Jan 4, 2019

View reviewed changes

dsgouda approved these changes Jan 4, 2019

View reviewed changes

dsgouda merged commit 3328ef7 into Azure:psSdkJson6 Jan 4, 2019

dsgouda removed the Changes Requested label Jan 4, 2019

Conversation

leonardbf commented Dec 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

shahabhijeet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsgouda left a comment

Choose a reason for hiding this comment

Uh oh!

dsgouda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leonardbf commented Dec 16, 2018 •

edited

Loading