[tcgc] add namespace flag
#2161
Conversation
iscai-msft
requested review from
lmazuel,
m-nash,
srnagar and
joheredi
as code owners
microsoft-github-policy-service
bot
added
the
lib:tcgc
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
iscai-msft
requested review from
bterlson,
markcowl,
allenjzhang and
timotheeguerin
as code owners
…cgc/namespaceFlag
| @@ -79,6 +79,7 @@ export async function createSdkContext< | |||
| decoratorsAllowList: [...defaultDecoratorsAllowList, ...(options?.additionalDecorators ?? [])], | |||
| previewStringRegex: options?.versioning?.previewStringRegex || tcgcContext.previewStringRegex, | |||
| disableUsageAccessPropagationToBase: options?.disableUsageAccessPropagationToBase ?? false, | |||
| namespace: context.options["namespace"], | |||
There was a problem hiding this comment.
I might be missing it but where are we blocking / ignoring this outside of azure flavor? @srnagar if I recall correctly that is what we were doing is azure only since thats the only use case that actually needs this?
There was a problem hiding this comment.
My understanding is we weren't going to block unbranded from using it, but we are really mainly adding it for Azure compatibility.
The general use case I have for this is:
- there's a
main.tspthat defines the service under one main namespace (saynamespace DocumentIntelligence) - there's a
client.tspwith client-customizations under one main namespace (saynamespace DocumentIntelligenceCustomizations) - we use
namespaceflag in tspconfig.yaml to give the azure name, sonamespace: Azure.DocumentIntelligence
There was a problem hiding this comment.
The namespace in tspconfig.yaml is mainly used for Azure and we will not use it for unbranded libraries. We just don't publicly document namespace option for unbranded libraries.
If we want to be very explicit, we can add a check in the TCGC that can be removed later if we want to extend this support for unbranded libraries.
There was a problem hiding this comment.
I personally feel like it's cleaner to keep it as is, and when we split up tcgc, we can keep namespace in the tcgc library. I think I'd almost prefer if we only want azure to use it, to call it something like azureNamespace. I don't feel very strongly about this though
There was a problem hiding this comment.
Yeah, I agree with just keeping it as is.
| const rootNamespaceName = diagnostics.pipe(getRootUserDefinedNamespaceName(context)); | ||
| // if you've defined a namespace flag, we replace the root name with the flag | ||
| return diagnostics.wrap( | ||
| override.replace(rootNamespaceName, context.namespace || rootNamespaceName), |
There was a problem hiding this comment.
better to judge undefined of context.namespace to prevent always find rootNamespaceName part.
| diagnostics.add( | ||
| createDiagnostic({ | ||
| code: "unclear-namespace-overriding", | ||
| messageId: "default", | ||
| format: {}, | ||
| target: context.program.getGlobalNamespaceType(), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
can we just report there is multiple user defined ns? i don't prefer couple this function with namespace override config.
Adding support for a
namespaceflag in tsp configs. The rules are simple:namespaceis present intspconfig.yaml, all instances of the root namespace will be replaced with the value ofnamespace.@clientNamespace. However, the client namespace will have to contain the root namespace