-
Notifications
You must be signed in to change notification settings - Fork 51
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] add flags docs #2216
base: main
Are you sure you want to change the base?
[tcgc] add flags docs #2216
Conversation
No changes needing a change description found. |
You can try these changes here
|
|
||
## 4. `flatten-union-as-enum` | ||
|
||
Whether you want TCGC to return unions of strings as enums. Default value is `true`. I feel this should not be a `tspconfig.yaml` flag, but an option you can pass to TCGC when calling `createSdkContext` |
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.
How do we configure it if it's not a flag?
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.
it can be passed in as an option to createSdkContext
which is called by the individual emitters. So emitters would look like createSdkContext(..., {flattenUnionAsEnum: false})
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.
That doesnt' reply to my question :). We made it a flag so users can decide the behavior they want, how do a user decide what behavior they want, if it's not through a flag?
|
||
## 3. `package-name` | ||
|
||
The `package-name` override you would like for your generated library. By default individual language emitters should calculate the package name for their library from the `SdkPackage.namespaces` that `tcgc` returns to them. |
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.
Feels like we may want to kill it?
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.
we have been talking about coalescing to package-name
across the js, csharp, and python libraries (java falls out of this). Would package-name
live in tcgc or would each language emitter have their own flag called package-name
? Now that I'm writing it out, I think it is the latter. Unless we switch our emitters to inherit our flags options from TcgcEmitterOptions
. We would then need to be more careful to make sure all options in TcgcEmitterOptions
apply to our core languages (or if they don't, we need to explicitly ok that)
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 think Java emitter uses all of them except package-name
flatten-union-as-enum
and emitter-name
would certainly affect TCGC behavior but I agree they seems not an emitter option.
|
||
When set to `true`, the emitter will generate low-level protocol methods for each service operation if `@convenientAPI` is not set for an operation. Default value is `true`. | ||
|
||
## 3. `package-name` |
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 wasn't aware we had a package-name
in TCGC, when I asks for microsoft/typespec#5758
Yeah, we can use this, if we agree there would be no need to setting a version.
Though unsure whether there would be another package-description
though.
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.
Python is fine to add package-version/package-description
or not since they are not necessary configurations for a python package.
But I think maybe we could add these configs in case users want to set them when calling emitter.
nit:
Python could consume package-verison
at here and package-description
at here.
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.
For now package name, version and description would be useful for JS. And in Azure scope the package name is required but others are optional.
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.
the package-name
will be soon deprecated in this pr: #2161
@@ -0,0 +1,35 @@ | |||
# TCGC `tspconfig.yaml` flags |
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'd like to clarify the concept of tcgc flags here, does that mean 1) tcgc would use these flags or 2) the common flags cross all emitters?
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.
any flag tcgc will use and get from emitter's config in tspconfig.yaml
.
@@ -0,0 +1,35 @@ | |||
# TCGC `tspconfig.yaml` flags | |||
|
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.
missing api-version
? is the flavor
in-scoped?
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.
Yes, api-version
flavor
probably will go away from emitter, as we should have 1 emitter for unbranded, and 1 emitter for azure. (maybe we need it later for unbranded on different org)
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.
oh yep! I can't believe I missed api-version
, thank you for the catch!
|
||
Default value is `none`. | ||
|
||
## 4. `flatten-union-as-enum` |
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.
another point is even we could expose this flag to customers that doesn't mean the customer's config would always take effective. it depends on how emitter handled them and is allowed to override or not. for example generate-protocol-methods is always true for js no matter what config yaml set.
|
||
## 6. `emitter-name` | ||
|
||
The name of the emitter you are calling TCGC from. TCGC uses this to keep track of what scope you belong to, and what information to set for you. I feel we should remove this from `tspconfig.yaml`. It is currently the second optional argument to pass to `createSdkContext`, which I think is the only place you should be able to pass it in |
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.
Agree, shouldn't be a flag
No description provided.