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.

Sign up for GitHub

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[tcgc] add flags docs #2216

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e830d6c
add docs
Feb 13, 2025
ab749aa
add doc for api-version and address some comments
Feb 14, 2025
e61cc1e
remove useless flag
tadelesh Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions packages/typespec-client-generator-core/design-docs/flags.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# TCGC `tspconfig.yaml` flags
Copy link
Member

@MaryGao MaryGao Feb 14, 2025 •
edited
Loading

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?

Copy link
Member

@tadelesh tadelesh Feb 17, 2025 •
edited
Loading

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.


Copy link
Member

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?

Copy link
Member

@weidongxu-microsoft weidongxu-microsoft Feb 14, 2025 •
edited
Loading

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)

Copy link
Contributor Author

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!

## 1. `generate-protocol-methods`

When set to `true`, the emitter will generate low-level protocol methods for each service operation if `@protocolAPI` is not set for an operation. Default value is `true`.

## 2. `generate-convenience-methods`

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`
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

@MaryGao MaryGao Feb 14, 2025 •
edited
Loading

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.

Copy link
Member

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


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.
Copy link
Member

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?

Copy link
Contributor Author

@iscai-msft iscai-msft Feb 14, 2025 •
edited
Loading

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)


Currently, emitters don't really use it, but it would be great to move language emitters to use this flag. Java should be excluded from this list because it doesn't apply to their naming structure.

Should remove any logic that derives namespace names from the value of `package-name`.

Default value is `none`.

## 4. `flatten-union-as-enum`
Copy link
Member

@MaryGao MaryGao Feb 14, 2025 •
edited
Loading

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.


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`
Copy link
Member

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?

Copy link
Contributor Author

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})

Copy link
Member

@lmazuel lmazuel Feb 14, 2025 •
edited
Loading

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?


## 5. `examples-dir`

Specifies the directory where the emitter will look for example files. If the flag isn’t set, the emitter defaults to using an `examples` directory located at the project root.

## 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 this should not be a `tspconfig.yaml` flag, but an option you can pass to TCGC when calling `createSdkContext`

## 7. `namespace`

If you want to override the namespaces set in the spec. In the process of adding, see [this](https://github.com/Azure/typespec-azure/pull/2161) PR. Affects the values in `SdkPackage.namespaces`.
Loading