Skip to content

Conversation

@avneeshrai
Copy link

@avneeshrai avneeshrai commented May 10, 2017

It's corresponding to new swagger spec which is under PR Azure/azure-rest-api-specs#1209
Intent is to get early feedback here.

Description


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

@avneeshrai,
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

1) Removing operationresults apis.
2) Introducing operations api.
3) Fixing issue in a patch model.
4) Changing few fields to string from boolean.
@avneeshrai avneeshrai changed the title [DO NOT MERGE] Adding projects for RecoveryServices.SiteRecovery service. Adding projects for RecoveryServices.SiteRecovery service. May 13, 2017
@avneeshrai
Copy link
Author

Can someone review this PR please. Corresponding swagger PR: Azure/azure-rest-api-specs#1209 is merged.

I have removed do not merge tag.

@@ -0,0 +1,47 @@
// Code generated by Microsoft (R) AutoRest Code Generator 0.16.0.0
// Changes may cause incorrect behavior and will be lost if the code is
Copy link
Contributor

Choose a reason for hiding this comment

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

@avneeshrai missing license header.
Chances are your swagger does not have the right License Header Constant, please double check, I believe the constant that you need to use in your swagger is "MIT-no-version"

Copy link
Author

Choose a reason for hiding this comment

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

@shahabhijeet can you please point to some example spec on what exacty needs to be done here
I tried searching MIT-no-version in azure-rest-api-spec repository but couldnt find any instance of it

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in the latest commit

@@ -0,0 +1,47 @@
// Code generated by Microsoft (R) AutoRest Code Generator 0.16.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@avneeshrai please use latest AutoRest (at least 1.0)

Copy link
Author

Choose a reason for hiding this comment

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

even after doing autorest --latest or autorest --reset
the version that i get is 0.16.0.0.
How to i get 1.0?

@avneeshrai
Copy link
Author

@shahabhijeet Please review and merge the change in swagger spec as you had suggested in the PR: Azure/azure-rest-api-specs#1227

@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Contributor

Choose a reason for hiding this comment

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

@avneeshrai please reference 1.5.0-preview of Resource manager in your test project.
With the new resource manager, you will be using the new nuget (might need to add new namespace) and then record your tests.
Don't worry if you see the older version of resource mangaer in your project, that is going to be there as it's a common dependency (we want to remove this dependency soon)
Record all your tests and that should solve the testing issue, I had mentioned earlier.

Copy link
Author

Choose a reason for hiding this comment

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

@shahabhijeet as discussed offline...we dont have dependency on Resource manager in our projects. Have removed all the stale references that we had.

@shahabhijeet shahabhijeet merged commit 4705103 into Azure:vs17Dev May 16, 2017
@avneeshrai avneeshrai deleted the vs17DR2 branch May 16, 2017 08:32
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