Skip to content

Azure Stack Admin for Commerce, Fabric, and InfrastructureInsights#3649

Closed
deathly809 wants to merge 9 commits intoAzure:psSdkJson6from
deathly809:psSdkJson6
Closed

Azure Stack Admin for Commerce, Fabric, and InfrastructureInsights#3649
deathly809 wants to merge 9 commits intoAzure:psSdkJson6from
deathly809:psSdkJson6

Conversation

@deathly809
Copy link
Copy Markdown
Member

Description

Initial pull request for Azure Stack Admin C# SDK containing Commerce, Fabric, and Infrastructure Insights.

Initial Swagger pull request: Azure/azure-rest-api-specs#1592


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

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.

@msftclas
Copy link
Copy Markdown

msftclas commented Sep 1, 2017

@deathly809,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

Copy link
Copy Markdown
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

Please have the common testbase be a separate nuget package and have all RPs take dependency on it.
have directory structure as below
src\AzureStack\TestCommon\Microsoft.AzureStack.Common.Test.csproj

Add < PackageReleaseNotes > and add Release notes.
I would recommend to use < ![CDATA and then format your Release notes

</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\..\SDKCommon\TestCommon\TestCommon.csproj" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@deathly809 have this be a nuget dependency instead of a project reference

</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\..\SDKCommon\TestCommon\TestCommon.csproj" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

have reference to nuget package

@@ -1,4 +1,5 @@
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="SdkCommon\dirs.proj" />
<Import Project="SDKs\dirs.proj" />
<Import Project="AzureStack\dirs.proj" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you do not need this file. Please remove this file.

<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<ItemGroup Label="SDKProject">
<ProjectToBuild Include="$(MSBuildThisFileDirectory)AzureStack\FabricAdmin\Fabric.Admin\Microsoft.AzureStack.Management.Fabric.Admin.csproj" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We no longer need this file

@shahabhijeet
Copy link
Copy Markdown
Contributor

@deathly809 closing this PR, as you will be opening two separate PRs. Please reopen this PR once you are ready to merge.

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