Skip to content

Enable the new cancellationToken pattern on non-azure libraries#4524

Merged
ArcturusZhang merged 20 commits intoAzure:feature/v3from
ArcturusZhang:enable-new-cancellationtoken-pattern
Apr 15, 2024
Merged

Enable the new cancellationToken pattern on non-azure libraries#4524
ArcturusZhang merged 20 commits intoAzure:feature/v3from
ArcturusZhang:enable-new-cancellationtoken-pattern

Conversation

@ArcturusZhang
Copy link
Copy Markdown
Member

@ArcturusZhang ArcturusZhang commented Apr 3, 2024

Fixes #4243

Description

Long story short: in non-azure libraries, convenience methods no longer have the parameter cancellationToken. Customers needing to provide a cancelling context should use the protocol methods instead for advanced scenarios.

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 force-pushed the enable-new-cancellationtoken-pattern branch from 691a1ba to fdce61a Compare April 3, 2024 09:20
@ArcturusZhang
Copy link
Copy Markdown
Member Author

@m-nash @annelo-msft this PR is ready to review

Copy link
Copy Markdown
Contributor

@annelo-msft annelo-msft left a comment

Choose a reason for hiding this comment

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

Approving with regard to the output code, which looks correct to me. I defer to others on the generator implementation.

Copy link
Copy Markdown
Member

@archerzz archerzz left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread test/UnbrandedProjects/Customized-TypeSpec/src/Generated/SuperClient.cs Outdated
@ArcturusZhang
Copy link
Copy Markdown
Member Author

The doc changes are distractions, I will extract those changes into a standalone PR and merge it first.

@ArcturusZhang
Copy link
Copy Markdown
Member Author

@annelo-msft FYI I updated this PR and now it only contains the change of removing cancellationToken (the other doc link change has been merged in another PR), it should be a lot more clear to review.

@ArcturusZhang

This comment was marked as outdated.

@ArcturusZhang ArcturusZhang merged commit 8210852 into Azure:feature/v3 Apr 15, 2024
@ArcturusZhang ArcturusZhang deleted the enable-new-cancellationtoken-pattern branch April 15, 2024 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable the new cancellation token pattern in the debranded generator

4 participants