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

C# SDK change with Hub's certificate and operation tasks #3847

Merged
merged 13 commits into from
Nov 20, 2017

Conversation

anusapan
Copy link
Contributor

@anusapan anusapan commented Nov 8, 2017

Description

Regenerated the SDK based off a new swagger and add/update the test cases for updated SDK.
Updated swagger PR


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.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

3 similar comments
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@msftclas
Copy link

msftclas commented Nov 8, 2017

CLA assistant check
All CLA requirements met.

NuGet.Config Outdated
<add key="NugetOfficialV3" value="https://api.nuget.org/v3/index.json" />
<add key="Local" value="tools/LocalNugetFeed" />
</packageSources>
<!-- To enable LocalFeed for testing uncomment the following line -->
Copy link
Member

Choose a reason for hiding this comment

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

@anusapan what is the need to edit this file? All this is doing is deleting spaces. Can you revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

@@ -35,5 +35,5 @@
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyVersion("1.0.0.1")]
[assembly: AssemblyFileVersion("1.0.0.1")]
[assembly: AssemblyVersion("1.0.0.2")]
Copy link
Member

Choose a reason for hiding this comment

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

@anusapan AssemblyVersion only changes when you update the Major version. Please do not update AssemblyVersion and make it same as AssemblyFileVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

<PackageTags>Microsoft Azure IotHub;IotHub management;IotHub;REST HTTP client;azureofficial;windowsazureofficial;netcore451511</PackageTags>
</PropertyGroup>
<PropertyGroup>
<TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
</PropertyGroup>
<PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

@anusapan you do not need this property anymore, so please remove it. If you get in the upstream changes, this will no longer be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

<PropertyGroup>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>

Copy link
Member

Choose a reason for hiding this comment

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

@anusapan please use latest version of Resource manager to record all your tests. You are still using 1.1.0, latest version is 1.6.0
YOU will have add reference to the new nuget, use the namespace from the new nuget and remove all traces of old namespace and then record your tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added new references of ResourceManager and recorded test again

@shahabhijeet
Copy link
Member

@azuresdkci test this please

@@ -5,7 +5,7 @@
<Description>Provides management capabilities for Microsoft Azure IotHub.</Description>
<AssemblyTitle>Microsoft Azure IotHub Management</AssemblyTitle>
<AssemblyName>Microsoft.Azure.Management.IotHub</AssemblyName>
<VersionPrefix>1.1.2</VersionPrefix>
<VersionPrefix>1.1.3</VersionPrefix>
<PackageTags>Microsoft Azure IotHub;IotHub management;IotHub;REST HTTP client;azureofficial;windowsazureofficial;netcore451511</PackageTags>
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added PackageReleaseNotes.

@shahabhijeet
Copy link
Member

@azuresdkci retest this please

@shahabhijeet
Copy link
Member

@azuresdkci test this please

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

2 similar comments
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@shahabhijeet shahabhijeet merged commit abc1f94 into Azure:psSdkJson6 Nov 20, 2017
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.

4 participants