-
Notifications
You must be signed in to change notification settings - Fork 231
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
base: main
Are you sure you want to change the base?
Conversation
fb24e3a
to
4e7b1e3
Compare
API change check APIView has identified API level changes in this PR and created following API reviews. Microsoft.Generator.CSharp.Input |
…e segment is the same as a client
packages/http-client-csharp/emitter/src/lib/client-model-builder.ts
Outdated
Show resolved
Hide resolved
badNamespaceSegments.add(lastSegment); | ||
|
||
Logger.getInstance().warn( | ||
`bad namespace ${client.clientNamespace} for client ${clientName}, please use @clientNamespace to specify a different namespace or @clientName to specify a different name for the client`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should promote @clientNamespace as this introduces multiple ways to rename things. @clientName can rename any object type.
See this issue for additional comments on this topic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean we should put @clientName
first here in the message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see - I removed the clientNamespace part.
need to add the extension namespace of the pipeline when invoking the extension method from the helpers. |
f1709bc
to
a544bef
Compare
using Client.Naming.Models; | ||
using NUnit.Framework; | ||
using ClientModel = Client.Naming.Models.ClientModel; | ||
// using System.Threading.Tasks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these test cases are removed because of this issue: #5653
@@ -50,6 +50,12 @@ const $lib = createTypeSpecLibrary({ | |||
default: paramMessage`${"message"}`, | |||
}, | |||
}, | |||
"bad-namespace": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "client-namespace-conflict"?
Fixes #5442
Fixes #5471
This PR contains the following:
removes thenamespace
configuration from the emitter because we no longer need it.removes themodel-namespace
configuration from the emitter because we no longer need it.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.
For 2 and 3, since it impacts azure generator, I would like to do them in a standalone PR.