Skip to content

add default message when the summary of a protocol/convenience method is missing#4592

Merged
ArcturusZhang merged 9 commits intoAzure:feature/v3from
ArcturusZhang:add-default-summary-when-absent
Apr 18, 2024
Merged

add default message when the summary of a protocol/convenience method is missing#4592
ArcturusZhang merged 9 commits intoAzure:feature/v3from
ArcturusZhang:add-default-summary-when-absent

Conversation

@ArcturusZhang
Copy link
Copy Markdown
Member

@ArcturusZhang ArcturusZhang commented Apr 17, 2024

Description

The issue this PR is trying to resolve was first found in this PR: #4573

Previously when an operation in tsp has no @doc decorator, we will generate nothing in summary of xml doc.
This works fine in our existing cases, but in the new pattern for non-azure libraries, a convenience method could have no parameters because it no longer takes cancellationToken.
Our CodeWriter is designed in this way: when we told it to write a xml tag summary but it turns out to write nothing like:

/// <summary> </summary>

it will remove this line as if we never let it to write this.
This would be a trouble if a method takes no parameters:

  • summary will not be written because it does not have one
  • parameters will not be written because it does not have one
  • exceptions will not be written because it does not throw exceptions (exceptions usually come from parameter validation)

And at the end, this public method is written without a xml doc, and this leads to a compilation error.

Therefore this PR introduces a default message for the operation, such as: The {name} method, so that we at least will write summary for a method.

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

@ArcturusZhang ArcturusZhang merged commit a06787a into Azure:feature/v3 Apr 18, 2024
@ArcturusZhang ArcturusZhang deleted the add-default-summary-when-absent branch April 18, 2024 05:31
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.

3 participants