Renames & Refactorings #9266
Renames & Refactorings #9266
Conversation
| @@ -107,7 +108,7 @@ export interface SearchRequest { | |||
| orderBy?: string; | |||
| /** | |||
| * A value that specifies the syntax of the search query. The default is 'simple'. Use 'full' if | |||
| * your query uses the Lucene query syntax. Possible values include: 'simple', 'full' | |||
| * your query uses the Lucene query syntax. Possible values include: 'Simple', 'Full' | |||
There was a problem hiding this comment.
it still feels like a codegen bug that the possible list is capitalized. If a user passes in 'Simple' instead of 'simple', would it still work? (I know TS might complain, but what happens if they're just using JS?)
/cc @joheredi
There was a problem hiding this comment.
TS will complain. JS will allow it
There was a problem hiding this comment.
Thanks for bringing this up Jeff, yes this is definitely a bug in Autorest.Typescriopt (v5). @sarangan12 do you know if the swagger changed here. Or were you fixing this by hand previously?
I can't find a recent change in Autorest.Typescript that may have regressed this. Anyway, I'll file a bug and make sure this is no happening in the new Autores.Typescript (v6)
There was a problem hiding this comment.
If JS allows it and the user sends it... does the server handle it correctly? If so, I have no problem here, otherwise I worry we'll get users into an error state they don't understand.
| fullOptions, | ||
| { | ||
| ...fullOptions, | ||
| searchText: searchText |
There was a problem hiding this comment.
This should always work, though technically we're supposed to honor whatever comes back from nextPageParameters, which includes searchText, which is being overridden here with the original value.
I think this is safe, but maybe @brjohnstmsft has feelings about it?
There was a problem hiding this comment.
But, this is a private method right. So, this should work
There was a problem hiding this comment.
Well the problem is the way paging work is the server sends us back the parameters to send for the next page... and it sends us all the parameters that it wants, which is the full query including searchText.
Technically it only modifies skip and top today, but we aren't supposed to rely on it.
I think this should be ok since I don't think the server would change the searchText between pages, even though it could.
There was a problem hiding this comment.
Seems unlikely we'd change searchText between pages, but in the long run it's better to follow the service's contract for paging. That means nextPageParameters takes precedence.
|
Oh one other thing I forgot @sarangan12 can you please update changelog with all the breaking changes? |
The changelog will be updated before release |
You should assume that enums are case-sensitive. |
This PR consists of code changes for the following tasks:
@xirzec Please review and approve
@ramya-rao-a FYI.....