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 .NET SDK from latest swagger for RecoveryServices.Backup #3311

Merged
merged 2 commits into from
Jun 15, 2017
Merged

Updated .NET SDK from latest swagger for RecoveryServices.Backup #3311

merged 2 commits into from
Jun 15, 2017

Conversation

DheerendraRathor
Copy link
Contributor

@DheerendraRathor DheerendraRathor commented Jun 1, 2017 •
edited
Loading

Description

  • Upgraded version to 2.0.0-preview
  • Removed redundant files
  • Added tests and record files for List Operations API
  • Updated ResourceManager to 1.6.0-preview

Swagger spec PR: Azure/azure-rest-api-specs#1267

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.

shahabhijeet
shahabhijeet suggested changes Jun 7, 2017
View reviewed changes
src/SDKs/RecoveryServices.Backup/Management.RecoveryServices.Backup/Generated/Models/Page2.cs
@@ -28,10 +28,10 @@ public class Page2<T> : IPage<T>
/// <summary>
/// Gets the link to the next page.
/// </summary>
[JsonProperty("")]
[JsonProperty("NextLink")]
Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor why is it not camelCase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is auto generated from swagger spec and which in turn reflects field name directly from service data contracts.

src/SDKs/RecoveryServices.Backup/Management.RecoveryServices.Backup/Generated/Models/Page2.cs
public string NextPageLink { get; private set; }

[JsonProperty("value")]
[JsonProperty("Value")]
Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor any reason this is not camelCase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for "NextLink"

...Management.RecoveryServices.Backup/Microsoft.Azure.Management.RecoveryServices.Backup.csproj Outdated
@@ -3,7 +3,7 @@
<PropertyGroup>
<PackageId>Microsoft.Azure.Management.RecoveryServices.Backup</PackageId>
<Description>Provides developers with libraries for the updated recovery services backup feature under Azure Resource manager.</Description>
<VersionPrefix>1.3.0-preview</VersionPrefix>
<VersionPrefix>1.4.0-preview</VersionPrefix>
Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor you have a breaking change, that requires major version change and not minor version.

Please also use latest resource manager nuget package for recording your tests that you have in this PR.
You are still using 1.1.0-preview Resource Manager and you need start using >=1.6.0

src/SDKs/RecoveryServices.Backup/Management.RecoveryServices.Backup/Properties/AssemblyInfo.cs Outdated
@@ -8,7 +8,7 @@
[assembly: AssemblyDescription("Provides management functionality for Microsoft Azure Recovery Services Backup Resources.")]

[assembly: AssemblyVersion("1.0.0.0")]
[assembly: AssemblyFileVersion("1.3.0.0")]
[assembly: AssemblyFileVersion("1.4.0.0")]
Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor major version change in FileVersion as well as AssemblyVersion

@DheerendraRathor
Copy link
Contributor Author

@shahabhijeet Currently project is referring Microsoft.Azure.ResourceManager 1.1.0-preview which is read from AzSdk.test.reference.props. On nuget this package is unavailable and name of package is Microsoft.Azure.Management.ResourceManager. Should I still refer to new package? I can't remove old 1.1.0 as it will require modification of AzSdk.test.reference.props and that will change references in all test projects.

@shahabhijeet
Copy link
Member

@DheerendraRathor you add the new package in your test project and not in reference.props file.
Version 1.1.0 and 1.6.0 can co-exists.
The idea is to make sure you are creating and using the latest resource manager package and using it in your tests.

@DheerendraRathor
Updated .NET SDK from latest swagger
e77137a 
- Upgraded version to 2.0.0-preview
- Removed redundant files
- Added tests and record files for List Operations API
- Updated ResourceManager to 1.6.0-preview
@DheerendraRathor
Copy link
Contributor Author

@shahabhijeet Can you please look into it? It has been pending for a while now.

@shahabhijeet shahabhijeet merged commit 93c8e2e into Azure:psSdkJson6 Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahabhijeet shahabhijeet shahabhijeet approved these changes

Assignees

@shahabhijeet shahabhijeet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DheerendraRathor @shahabhijeet @msftclas