Skip to content

Regenerating preview SDK based off of latest swagger spec update#5203

Merged
brjohnstmsft merged 12 commits intoAzure:search-previewfrom
arv100kri:skillset-configuration
Feb 4, 2019
Merged

Regenerating preview SDK based off of latest swagger spec update#5203
brjohnstmsft merged 12 commits intoAzure:search-previewfrom
arv100kri:skillset-configuration

Conversation

@arv100kri
Copy link
Copy Markdown
Member

@arv100kri arv100kri commented Feb 1, 2019

Description

  1. Regenerate the preview SDK off of the changes to the swagger specifications from this PR
  2. Updated a skillset test to account for stricter validation in DefaultLanguageCode parameter for OcrSkill
  3. Updated DataSourceTests to also account for stricter validation
  4. Renamespace SearchManagementClientTests

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
Copy Markdown
Member

@brjohnstmsft brjohnstmsft left a comment

Choose a reason for hiding this comment

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

We need to discuss potential backcompat concerns before I can merge this. Otherwise, the changes look good.

@brjohnstmsft
Copy link
Copy Markdown
Member

brjohnstmsft commented Feb 2, 2019

I missed it in my original review, but could you please also regenerate the code for Microsoft.Azure.Search.Data? #Resolved

@arv100kri
Copy link
Copy Markdown
Member Author

yup, I've done that. Commit 797faef9823b453f4130d3ac4288f047bb84c8a5


In reply to: 459909441 [](ancestors = 459909441)

Copy link
Copy Markdown
Member

@brjohnstmsft brjohnstmsft left a comment

Choose a reason for hiding this comment

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

Had a question about a credential in a test; Otherwise LGTM.

Copy link
Copy Markdown
Member

@brjohnstmsft brjohnstmsft left a comment

Choose a reason for hiding this comment

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

@arv100kri Changes look good. We just need to wait for your Swagger change to be merged so you can regenerate one more time to update the metadata, and then I'll merge this.

@brjohnstmsft
Copy link
Copy Markdown
Member

@arv100kri Apparently I'm a bit behind. I see the Swagger has been merged, and the metadata indicates that the regenerated code was based off Azure/master. I'm going to merge this now.

@brjohnstmsft brjohnstmsft merged commit fc23525 into Azure:search-preview Feb 4, 2019
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.

2 participants