-
Notifications
You must be signed in to change notification settings - Fork 67
Add typespec-metadata emitter #3734
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
base: main
Are you sure you want to change the base?
Conversation
|
All changed packages have been documented.
Show changes
|
3893d86 to
a426004
Compare
commit: |
|
You can try these changes here
|
117fbad to
badc240
Compare
| const includedEmitters = [ | ||
| "@azure-tools/typespec-python", | ||
| "@azure-tools/typespec-java", | ||
| "@azure-tools/typespec-csharp", |
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.
Should we use a naming pattern instead of a hard-coded list? This already doesn't look complete as I know we have newer emitters for csharp.
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 figured an include list made more sense than an exclude list. Otherwise it will collect information for any emitters in tsp-config including ones we don't care (or possibly know) about.
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.
Does it hurt to collect for all emitters as long as the properties match the pattern we are looking for?
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.
Arguably no, but if it's filled with weird random stuff, I figured it would be confusing for consumers.
| * Rust-specific metadata parser. | ||
| * Uses crate-name as the primary identifier. | ||
| */ | ||
| function parseRust( |
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.
Is there a good way to share these with the language specific emitter? So, both are in-sync?
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 emitters aren't libraries, so AFAIK no, but we could centralize the logic in TCGC, which is the library those emitters depend on (though obviously not for this logic or it would already be there). I'm not sure the added complexity makes sense since any external tool that wants this info would go through this emitter anyhow, so this becomes the single point of reference. I'm also not sure TCGC would welcome language-specific logic in it, since it's purpose is (AFAIK) to provide language agnostic library support.
But if there was any other place this logic should go, it's there.
c9ce364 to
39cc351
Compare
39cc351 to
198d93e
Compare
Closes Azure/azure-sdk-tools#13302
Here's an example output: