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

regenerate the sdk since the swagger is updated #4136

Merged
merged 7 commits into from
Mar 20, 2018

Conversation

jeffrey-ACE-zz
Copy link
Contributor

@jeffrey-ACE-zz jeffrey-ACE-zz commented Mar 15, 2018

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.

Copy link
Member

@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 add a link to your swagger spec PR.
Also please pull latest from upstream and use generate.cmd to generate your SDK.
Finally build after generation using
msbuild build.proj /t;build /p:Scope=SDKs\ManagementPartner

this will ensure all the meta data is generated required for the SDK to be merged.

@jeffrey-ACE-zz
Copy link
Contributor Author

@shahabhijeet
Swagger spec PR: Azure/azure-rest-api-specs#2665
Pulled from upstream.
For the other two steps, run generate.cmd and build, I did these too, no new change

@shahabhijeet
Copy link
Member

@jeffrey-ace in order to publish, you will have to bump up the version as well as add ReleaseNotes.

@jeffrey-ACE-zz
Copy link
Contributor Author

@shahabhijeet thank you, made the fix.

@shahabhijeet
Copy link
Member

@jeffrey-ace you have to generate SDK using generate.cmd or generate.ps1
We cannot accept this PR without it.

@shahabhijeet shahabhijeet requested a review from dsgouda March 19, 2018 19:20
@dsgouda
Copy link
Contributor

dsgouda commented Mar 19, 2018

@jeffrey-ace Please create a generate.ps1 script next to your generate.cmd and run it, this should generate the same code but with a diff in the .txt file. Please commit these changes

@jeffrey-ACE-zz
Copy link
Contributor Author

@dsgouda Created the generate.ps1 and run it, but didn't see any new change. can you please take a look?

@dsgouda
Copy link
Contributor

dsgouda commented Mar 19, 2018

@jeffrey-ace that was expected, we are moving towards replacing the .cmd file with a .ps1 script without affecting the code generation.
Your changes look good.

@jeffrey-ACE-zz
Copy link
Contributor Author

@dsgouda cool, thanks, can you please also approve and merge this PR

@@ -0,0 +1 @@
powershell.exe -ExecutionPolicy Bypass -NoLogo -NonInteractive -NoProfile -File "$(split-path $SCRIPT:MyInvocation.MyCommand.Path -parent)\..\..\..\..\tools\generateTool.ps1" -ResourceProvider "managementpartner/resource-manager" -SdkDirectory "$(split-path $SCRIPT:MyInvocation.MyCommand.Path -parent)" -PowershellInvoker -AutoRestVersion "latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

@jeffrey-ace Please remove the -SdkDirectory argument here and generate again.
Apologize for the confusion, will fix the example soon.

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Thanks @jeffrey-ace LGTM subject to CI builds, will merge on passing.

@jeffrey-ACE-zz
Copy link
Contributor Author

@dsgouda , all checks have passed, can you please 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.

4 participants