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.

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

dpg, integrate TCGC getAllModels - basic support #2564

Closed
23 of 26 tasks
Tracked by #606
haolingdong-msft opened this issue Feb 21, 2024 · 8 comments · Fixed by #2698
Closed
23 of 26 tasks
Tracked by #606

dpg, integrate TCGC getAllModels - basic support #2564

haolingdong-msft opened this issue Feb 21, 2024 · 8 comments · Fixed by #2698
Assignees
Labels

Comments

@haolingdong-msft
Copy link
Member

haolingdong-msft commented Feb 21, 2024

TCGC getAllModels() code is already on main. We will need to getAllModels() in emitter to replace the logics in processModels().

Tasks

Preview Give feedback
  1. 0 of 1
    haolingdong-msft
  2. 2 of 2
    haolingdong-msft
  3. haolingdong-msft

Possible gaps between TCGC type and compiler type:

  1. unkown type in compiler will be converted to any type in TCGC SdkType.
  2. void and never is not supported in TCGC SdkType yet. TCGC issue: Enhance typing for intrinsic type typespec-azure#292
  3. SdkType does not contain namespace information. Model and enum type from getAllModels does not have namespace typespec-azure#290
  4. SdkType.description and SdkType.detail: If both @summary and @doc exist, .description will be the content in @summary and detail will be the content in @doc. If only @doc exists, then description will be the content in @doc.
  5. TCGC types does not have concept of @format, it will map the type with @format to an SdkBuiltInType. (expected)
  6. Optional literal do not have correct generated name. [TCGC][Bug] getAllModels, Optional literal do not have correct generated name typespec-azure#553
  7. Description is not correctly returned for scalar types [TCGC][Bug] getAllModels, description is not correctly returned for scalar types typespec-azure#555
  8. discriminator, Returned model properties are not correct when discriminator as @clientName decorator [TCGC][Bug] getAllModels, discriminator, Returned model properties are not correct when discriminator has @clientName decorator typespec-azure#554
  9. model is array case not handled correctly: [TCGC][Bug] model is array case not handled correctly typespec-azure#690
  10. Type of model property/enum member when it is defined as constant number: Type of model property/enum member when it is defined as constant number microsoft/typespec#3314
  11. when should standalone models be generated: [TCGC][Discussion] model types, when should standalone models be generated typespec-azure#823
  12. ARM breaking change on UserAssignedIdentity: [TCGC] [ARM] Expected client code on definition model UserAssignedIdentities is Record<UserAssignedIdentity>;  typespec-azure#825

Log Time cost: 10-12 weeks

TCGC dependencies

Preview Give feedback
  1. bug getAllModels lib:tcgc
    tadelesh
  2. feature lib:tcgc
    ArcturusZhang
  3. getAllModels lib:tcgc
    iscai-msft
  4. getAllModels lib:tcgc
    tadelesh
  5. getAllModels lib:tcgc
    tadelesh
  6. haolingdong-msft
  7. MPG Impact lib:tcgc needs-area needs-team-attention
    allenjzhang haolingdong-msft
  8. lib:tcgc needs-area
@haolingdong-msft haolingdong-msft self-assigned this Feb 21, 2024
@weidongxu-microsoft
Copy link
Member

The Model is get from the raw Type. Use raw in it is meaningless.

@lirenhe
Copy link
Member

lirenhe commented Feb 23, 2024

The Model is get from the raw Type. Use raw in it is meaningless.

Please try to use the real models returned by TCGC instead of raw. And we shall continue reducing the usage of 'raw' in the source code till it is no longer needed.

cc @lmazuel

@weidongxu-microsoft
Copy link
Member

The Model is get from the raw Type. Use raw in it is meaningless.

Please try to use the real models returned by TCGC instead of raw. And we shall continue reducing the usage of 'raw' in the source code till it is no longer needed.

cc @lmazuel

I was saying the same thing... "Use raw in it is meaningless."

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Feb 23, 2024

never means the whole thing not exist (property: never means no such property). So it is correct that it not map to anything.

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Feb 29, 2024

Personally opinion:
You can start (incrementally) merge back to main branch, if you can handle Scalar/Record/Array and Model, preferrable Enum as well (they are major types -- it be fine at this stage to use raw if a nested ModelProperty is e.g. of type Union)

@haolingdong-msft
Copy link
Member Author

I agree, I will try to make the change incrementally and split to seperate mergable pr.

@haolingdong-msft
Copy link
Member Author

Loop page

@haolingdong-msft
Copy link
Member Author

TCGC will deprecate nullable property on type. Only property, dictionary value and additional property has nullable property.

@haolingdong-msft haolingdong-msft changed the title dpg, integrate TCGC getAllModels dpg, integrate TCGC getAllModels - basic support Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants