-
Notifications
You must be signed in to change notification settings - Fork 45
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
[tcgc] Namespace support #1786
[tcgc] Namespace support #1786
Conversation
* namespace Contoso; | ||
* ``` | ||
*/ | ||
extern dec clientNamespace( |
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.
current impl for @clientNamespace
will need author to use full qualified name to do the namespace renaming.
@@ -691,6 +699,17 @@ export interface SdkPackage<TServiceOperation extends SdkServiceOperation> { | |||
enums: SdkEnumType[]; | |||
unions: (SdkUnionType | SdkNullableType)[]; | |||
crossLanguagePackageId: string; | |||
namespaces: SdkNamespace<TServiceOperation>[]; |
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.
instead of providing helper function, i put the namespaces hierarchy info into the current SdkPackage
type and introduced another SdkNamepace
type to wrapper the namespace contents.
All changed packages have been documented.
|
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.
Overall LGTM.
Though we probably still need more discussion on namespace on template instances.
We can start with this PR (and a patch release), and iterate upon it if necessary.
PS: do we need to update all cadl-ranch cases? I guess Java would had to add @clientNamespace
to all of them (about the com.
prefix) -- not sure about other languages.
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.
OK for me as a starter.
You can try these changes here
|
@iscai-msft do you have any concern for this pr? i supposed to make it in the nov release, but obviously i missed it. so, i chagned to merge to release branch and will release a hotfix with the new ns support. language's emitters depend on this. |
i'll do a release of this today @tadelesh |
resolve: #1604
resolve: #1722
Usage
clientNamespace
property fromSdkModelType
SdkEnumType
SdkNullableType
SdkUnionType
namespaces
property fromSdkPackage
, this will returnSdkNamespace[]
which contains the root namespaces, eachSdkNamespace
contains clients/models/enums/unions/namespaces belong to this namespace.