Skip to content

Put all optional client parameters into client options#3910

Merged
pshao25 merged 25 commits intoAzure:feature/v3from
pshao25:constructor3706
Nov 28, 2023
Merged

Put all optional client parameters into client options#3910
pshao25 merged 25 commits intoAzure:feature/v3from
pshao25:constructor3706

Conversation

@pshao25
Copy link
Copy Markdown
Member

@pshao25 pshao25 commented Nov 7, 2023

  1. Change constructor appearance to Our generator creates client libraries which violate AZC0006 and AZC0007 #3706 (comment)
  2. Fix the problem causing from M4, i.e., when there is no endpoint set in swagger, M4 would still claim it is an optional one with default "", which is expected as a required one.

Now constructor will only have required parameters, all the optional parameters except endpoint will go to client options.

Regen: Azure/azure-sdk-for-net#40292

Comment thread src/AutoRest.CSharp/Common/Output/Models/Types/ClientOptionsTypeProvider.cs Outdated
Comment thread src/AutoRest.CSharp/LowLevel/Output/DpgOutputLibraryBuilder.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Output/Models/Types/ClientOptionsTypeProvider.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Output/Models/Shared/Parameter.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Output/Models/Shared/Parameter.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Generation/Writers/ClientOptionsWriter.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Output/Models/Shared/Parameter.cs
Comment thread src/AutoRest.CSharp/Common/Output/Models/Types/ClientOptionsTypeProvider.cs Outdated
Comment thread src/AutoRest.CSharp/LowLevel/Generation/DpgClientWriter.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Generation/Writers/ClientOptionsWriter.cs Outdated
Comment thread test/AutoRest.TestServerLowLevel.Tests/url-path-items.cs Outdated
Comment thread src/AutoRest.CSharp/Common/Generation/Writers/ClientOptionsWriter.cs Outdated
Comment thread src/AutoRest.CSharp/LowLevel/Output/DpgOutputLibraryBuilder.cs
Comment thread src/AutoRest.CSharp/LowLevel/Generation/DpgClientWriter.cs Outdated
Comment thread test/AutoRest.TestServerLowLevel.Tests/url-path-items.cs
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Copy link
Copy Markdown
Member

@ArcturusZhang ArcturusZhang left a comment

Choose a reason for hiding this comment

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

We should avoid the usage of the word user because it is kind of ambiguous - it could be the user of the generator or the user of the generator.
We could use passive phrase to avoid that

Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
Comment thread docs/features/service_client_constructors.md Outdated
@pshao25 pshao25 merged commit 9b54a18 into Azure:feature/v3 Nov 28, 2023
@pshao25 pshao25 deleted the constructor3706 branch November 28, 2023 09:25
live1206 pushed a commit to live1206/autorest.csharp that referenced this pull request Dec 11, 2023
* Put all optional client parameters into client options

* update

* Regenerate all code (any successful run even without change will have a new commit)

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

---------

Co-authored-by: actions-user <action@github.com>
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.

6 participants