Azure / azure-sdk

This is the Azure SDK parent repository and mostly contains documentation around guidelines and policies as well as the releases for the various languages supported by the Azure SDK.
http://azure.github.io/azure-sdk
MIT License
477 stars 295 forks source link

Board Review: Azure Cognitive Search SDK for Java - preview 2 #791

Closed CodeRunRepeat closed 1 year ago

CodeRunRepeat commented 4 years ago

The Basics

About this client library

Artifacts required (per language)

Champion Scenarios

Scenario 1: Setting up a basic search solution

This scenario assumes an existing search service

Scenario 2: Running a search solution

This scenario assumes an existing search solution as set up in scenario 1

Scenario 3: Refining search capabilities

This scenario assumes an existing search solution as set up in scenario 1

Agenda for the review

A board review is generally split into two parts, with additional meetings as required

Part 1 - Introducing the board to the service:

After part 1, you may schedule additional meetings with architects to refine the API and work on implementation.

Part 2 - the "GA" meeting

Thank you for your submission

CodeRunRepeat commented 4 years ago

@JonathanGiles @brjohnstmsft we have now closed all the bugs that were captured as part of this review. The new revision of the API is available at https://apiview.azurewebsites.net/Assemblies/Revisions/2abf935de65e4187964bea5b817872d6.

Here is the full list of related work items. Please let us know what else is needed to close the review. cc @adrianhall

Work Item Title Tags Reason PRs
Bug 1626 Consider reshaping Analyzer and derived classes #adp; #autorest Deferred  
Bug 1631 Review ConditionalSkill class #adp; #autorest_config As Designed  
Bug 1698 DataSources static methods naming #adp; blocked Fixed and verified  
Bug 1615 Review "autocomplete" naming vs. "autoComplete" #adp; #swagger As Designed  
Bug 1627 Review naming for AutocompleteOptions.getTop()/setTop() #adp; #autorest_config As Designed  
Bug 1612 Review use case and base type for Document() #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/306
Bug 1619 Review overloads in service plane methods wrt "withResponse" (async client) #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/314
Bug 1620 Use datasource or dataSource consistently throughout methods and parameters #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/307
Bug 1632 Consider renaming IndexGetStatisticsResult + model classes #adp; #swagger Deferred  
Bug 1678 Add createDataSource methods to SearchService(Async)Client #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/326
Bug 1613 JavaBeans naming for RangeFacetResult properties #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1616 Review the need for "withResponse" "overloads" in autocomplete #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/314
Bug 1621 JavaBeans naming for ValueFacetResult properties #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1623 JavaBeans naming for SearchPagedResponse properties #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1624 JavaBeans naming for SuggestPagedResponse properties #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1629 Research overlap between generated AutocompleteOptions and AutocompleteRequest classes #adp; #autorest_config As Designed  
Bug 1622 Review static DataSources methods that create a new DataSource #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/330
Bug 1625 Consider hiding AnalyzeResult and AutocompleteResult #adp; #autorest_config Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/338
Bug 1630 Rename CognitiveServicesByKey #adp; #swagger Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/338
        https://github.com/Azure/azure-rest-api-specs/pull/7859
Bug 1618 Implement required settings in SearchIndexClientBuilder #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/334
Bug 1614 In Search()()Client classes, change getApiVersion() #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/334
Bug 1617 Consider renaming ApiKeyCredentials #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/329
Bug 1696 Change references to Cosmos DB to Cosmos #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/331
Bug 1697 Ensure there aren't any public classes that are not part of the public API #adp Verified https://github.com/navalev/azure-sdk-for-java-pr/pull/338

ADP2 report.xlsx

brjohnstmsft commented 4 years ago

Looks good to me, except for one item: Bug 1632 | Consider renaming IndexGetStatisticsResult + model classes

I've made the Swagger change for that in this PR. Please review and re-generate the Java SDK code accordingly.

CodeRunRepeat commented 4 years ago

Thanks @brjohnstmsft will do as soon as our repo is resurrected.

navalev commented 4 years ago

@brjohnstmsft code has been regenerated following the swagger spec changes as per https://github.com/navalev/azure-sdk-for-java-pr/pull/349

CodeRunRepeat commented 4 years ago

@brjohnstmsft @JonathanGiles can we close this? cc @navalev

brjohnstmsft commented 4 years ago

I'm fine closing it if @JonathanGiles is.