Skip to content
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.

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

refactor(generator): remove null check on client description #4828

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

archerzz
Copy link
Member

This is a prerequisite change for #3994 The null check for client description is meaningless, because ClientBuilder will add a default description anyway:

public static string CreateDescription(string description, string clientPrefix)
=> string.IsNullOrWhiteSpace(description)
? $"The {clientPrefix} service client."
: BuilderHelpers.EscapeXmlDocDescription(description);
Right now in our emitter, we will set description to an empty string:
clientDesc = getDoc(program, container) ?? "";
But in TCGC, the description will be null.

So, let's remove this meaningless check.

Description

Add your description here!

Checklist

To ensure a quick review and merge, please ensure:

  • The PR has a understandable title and description explaining the why and what.
  • The PR is opened in draft if not ready for review yet.
    • If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
  • The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

  • Build is completely green
    • Submissions with test failures require tracking issue and approval of a CODEOWNER
  • At least one +1 review by a CODEOWNER
  • All -1 reviews are confirmed resolved by the reviewer
    • Override/Marking reviews stale must be discussed with CODEOWNERS first

This is a prerequisite change for Azure#3994 The null check for client description is meaningless, because `ClientBuilder` will add a default description anyway: https://github.com/Azure/autorest.csharp/blob/4d55b600ee877e68675297546e70a765fb679039/src/AutoRest.CSharp/Common/Output/Builders/ClientBuilder.cs#L28-L31
Right now in our emitter, we will set `description` to an empty string: https://github.com/Azure/autorest.csharp/blob/4d55b600ee877e68675297546e70a765fb679039/src/TypeSpec.Extension/Emitter.Csharp/src/lib/client-model-builder.ts#L281
But in TCGC, the `description` will be null.

So, let's remove this meaningless check.
@archerzz
Copy link
Member Author

By the way, it is possible to have a client without description. See

a namespace can have no @summary

@archerzz archerzz enabled auto-merge (squash) June 20, 2024 07:17
@archerzz archerzz merged commit c875f2e into Azure:feature/v3 Jun 20, 2024
8 checks passed
@archerzz archerzz deleted the fix/client-description-null-check branch June 20, 2024 07:19
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