Skip to content

[Search] Rename types for better SDK code #16039

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.

Sign up for GitHub

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

Merged
lmazuel merged 12 commits into from
Sep 28, 2021
Merged

[Search] Rename types for better SDK code #16039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fffc0b8
Rename types for better SDK code
Mohit-Chakraborty Sep 14, 2021
d543551
Merge remote-tracking branch 'origin/main' into mohitc/SearchSdkRenames
Mohit-Chakraborty Sep 14, 2021
52fd4a9
Rename Speller and Captions search options
Mohit-Chakraborty Sep 15, 2021
95c74c1
Reorder parameters to have consistency across APIs
Mohit-Chakraborty Sep 15, 2021
9cd36f7
Remove 'SearchIndexer' prefix from projection selectors
Mohit-Chakraborty Sep 20, 2021
5c34638
Change enum name AdlsGen2->AzureDataLakeStorageGen2
Mohit-Chakraborty Sep 21, 2021
964d6b3
move additional .NET SDK renames to the REST spec
Mohit-Chakraborty Sep 23, 2021
67ac832
Revert changes in KnowledgeStore which has GAed
Mohit-Chakraborty Sep 23, 2021
d034a2e
Revert changes in CustomEntityLookupSkill which has GAed
Mohit-Chakraborty Sep 23, 2021
12451c8
Revert changes in SIKSBlobProjectionSelector which has GAed
Mohit-Chakraborty Sep 23, 2021
7c332cd
Revert changes in SearchIndexerDataSourceType which has GAed
Mohit-Chakraborty Sep 23, 2021
64569c4
Rename ignoreResetRequirements->skipIndexerResetRequirementForCache
Mohit-Chakraborty Sep 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions specification/search/data-plane/Azure.Search/preview/2021-04-30-Preview/searchindex.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@
"lexicon"
],
"x-ms-enum": {
"name": "Speller",
"name": "QuerySpellerType",
Copy link
Member

@tg-msft tg-msft Sep 21, 2021

Choose a reason for hiding this comment

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

Does Type add anything to this name? Most enums are better without an extra Type at the end.

Copy link
Contributor Author

@Mohit-Chakraborty Mohit-Chakraborty Sep 22, 2021

Choose a reason for hiding this comment

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

You are right - in our discussion we only decided on QueryAnswerType rename for the Answers parameter.
We have 3 params that are similar in nature and usage and it would be good to keep them consistently named - Speller->QuerySpellerType, Answers->QueryAnswerType and Captions->QueryCaptionType - all extensible enums.

One pro of adding the suffix Type is that the param can otherwise be misunderstood as containing part of a result object. It is best illustrated for QueryAnswer and QueryCaption. We could leave the third as QuerySpeller or tack on Type there too to make all 3 names closely grouped.

Copy link
Contributor Author

@Mohit-Chakraborty Mohit-Chakraborty Sep 27, 2021

Choose a reason for hiding this comment

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

@tg-msft - what do you think?
Also, do you know how should I go forward with this PR with the validation errors regarding changes in an existing swagger? Do I need to send an email, or can I tag folks here?

"modelAsString": true,
"values": [
{
Expand All @@ -1479,7 +1479,7 @@
"extractive"
],
"x-ms-enum": {
"name": "Answers",
"name": "QueryAnswerType",
"modelAsString": true,
"values": [
{
Expand All @@ -1503,7 +1503,7 @@
"extractive"
],
"x-ms-enum": {
"name": "Captions",
"name": "QueryCaptionType",
"modelAsString": true,
"values": [
{
Expand Down
9 changes: 5 additions & 4 deletions specification/search/data-plane/Azure.Search/preview/2021-04-30-Preview/searchservice.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,10 @@
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/DisableCacheReprocessingChangeDetectionParameter"
"$ref": "#/parameters/IgnoreResetRequirementsParameter"
},
{
"$ref": "#/parameters/IgnoreResetRequirementsParameter"
"$ref": "#/parameters/DisableCacheReprocessingChangeDetectionParameter"
}
],
"x-ms-request-id": "request-id",
Expand Down Expand Up @@ -814,10 +814,10 @@
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/DisableCacheReprocessingChangeDetectionParameter"
"$ref": "#/parameters/IgnoreResetRequirementsParameter"
},
{
"$ref": "#/parameters/IgnoreResetRequirementsParameter"
"$ref": "#/parameters/DisableCacheReprocessingChangeDetectionParameter"
}
],
"x-ms-request-id": "request-id",
Expand Down Expand Up @@ -10033,6 +10033,7 @@
},
"IgnoreResetRequirementsParameter": {
"name": "ignoreResetRequirements",
"x-ms-client-name": "skipIndexerResetRequirementForCache",
"in": "query",
"required": false,
"type": "boolean",
Expand Down