Skip to content

Conversation

@solankisamir
Copy link
Member

@solankisamir solankisamir commented Aug 10, 2018

Description

Azure/azure-rest-api-specs#3614
Azure/azure-rest-api-specs#3609
Azure/azure-rest-api-specs#3430


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.

@solankisamir
Copy link
Member Author

@shahabhijeet looks to be some issue with infra

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.

Looks great apart from the comment

@@ -1 +1 @@
Start-AutoRestCodeGeneration -ResourceProvider "apimanagement/resource-manager" -AutoRestVersion "latest"
Start-AutoRestCodeGeneration -ResourceProvider "apimanagement/resource-manager" -AutoRestVersion "latest" -AutoRestCodeGenerationFlags "--tag=package-2018-01"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please set this tag as the default tag in the REST spec. Is there a reason why you are picking a particular tag here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we want the documentation to be generated off the -preview version, but the SDK to be targeted for specific stable version.

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC documentation is generated from the published nuget package, so SDK and documentation should ideally be the same version
@shahabhijeet could you please take a look

@shahabhijeet
Copy link
Contributor

@solankisamir if your purpose is to generate against selective tag, then that selective tag should be the default tag in your Readme.md.
Basically you have tag1, tag2, tag3 for your various versions of spec files.
If you have decided to generate SDK against tag2, let tag2 be the default tag that all future SDKs will be generated, until you change it to something else.
You need to tie your intention to generate SDK in the rest spec repository Readme.md than generate.ps1 file in SDK repo.
Also how are JAVA, python SDKs suppose to get generate, who is keeping track as to which tag are they using?
This will cause a lot of confusion when we detect a security issue and have to regenerate and publish SDKs on a short notice.

@solankisamir
Copy link
Member Author

@dsgouda updated the SDK generation to be agnostic of version.

@solankisamir
Copy link
Member Author

@shahabhijeet @dsgouda a gentle ping on this!

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.

LGTM

@dsgouda
Copy link
Contributor

dsgouda commented Aug 16, 2018

Apologize for the delay

@dsgouda dsgouda merged commit a6ff07c into Azure:psSdkJson6 Aug 16, 2018
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