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

Implement tsp namespace for http-client-csharp #5443

Merged
merged 70 commits into from
Feb 7, 2025

Conversation

ArcturusZhang
Copy link
Member

@ArcturusZhang ArcturusZhang commented Dec 25, 2024

Fixes #5442
Fixes #5471
Fixes #5563
Fixes Azure/azure-sdk-for-net#47670

This PR contains the following:

  1. now we honor the namespace defined in typespec for both model and clients (short for "namespace is namespace" feature)
  2. removes the namespace configuration from the emitter because we no longer need it. Azure generator still needs this, we will add it back in azure emitter (temporary until azure generator decides how to deal with it)
  3. removes the use-model-namespace configuration from the emitter because we no longer need it. Azure generator still needs this, we will add it back in azure emitter (temporary until azure generator decides how to deal with it)
  4. reports diagnostic when there are conflicts between namespace segments and subclient names.

Because this change changes the namespace of literally everything, there are quite a few file changed. Majority of them are the generated files changed for the namespace.

We also have a piece of logic that automatically renames the client Models to ModelsOps in our emitter. We should not have it in the emitter any way. This part is removed. Azure part needs it, therefore we will add the same logic back in azure generator.

Update:
I have to add the configuration namespace back because autorest.csharp is still strongly coupled with this configuration, removing it causes massive changes on that side. Therefore I decided that we keep it here for now, we still write its value to configuration.json for backward compatibility, but MGC will not read it nor use it.

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Dec 25, 2024
@azure-sdk
Copy link
Collaborator

azure-sdk commented Dec 25, 2024

API change check

APIView has identified API level changes in this PR and created following API reviews.

Microsoft.Generator.CSharp.Input
@typespec/http-client-csharp
Microsoft.Generator.CSharp
Microsoft.Generator.CSharp.ClientModel

@ArcturusZhang

This comment was marked as outdated.

Copy link
Contributor

@JoshLove-msft JoshLove-msft left a comment

Choose a reason for hiding this comment

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

Based on the discussion in the DPG Redmond Sync channel, it is sounding like we will need to support the namespace override from tspconfig.yaml.

@JoshLove-msft
Copy link
Contributor

JoshLove-msft commented Jan 23, 2025

Based on the discussion in the DPG Redmond Sync channel, it is sounding like we will need to support the namespace override from tspconfig.yaml.

Nevermind - the tspconfig override only needs to be supported in the Azure plugin - not for unbranded.

@JoshLove-msft
Copy link
Contributor

Please file the follow up issue to remove namespace from MGC emitter options along with the steps that would need to be taken in autorest.csharp to upgrade. Ideally, we could just remove it and then leave autorest.csharp pinned to an earlier version.

@ArcturusZhang ArcturusZhang added this pull request to the merge queue Feb 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 7, 2025
@ArcturusZhang ArcturusZhang added this pull request to the merge queue Feb 7, 2025
Merged via the queue into microsoft:main with commit 6210262 Feb 7, 2025
21 checks passed
@ArcturusZhang ArcturusZhang deleted the implement-namespace branch February 7, 2025 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp
Projects
None yet
4 participants