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

Jump to bottom

Fix the isUnexpected narrowing issue in modular #2654

Merged
merged 43 commits into from
Jul 23, 2024
Merged

Fix the isUnexpected narrowing issue in modular #2654

merged 43 commits into from
Jul 23, 2024

Conversation

MaryGao
Copy link
Member

@MaryGao MaryGao commented Jul 9, 2024 •
edited
Loading

fixes #2502

see design doc

We decided to go with option 1 - to ignore this issue and castcade this in modular side.

So we will do narrowing with two cases:

  • [existing] LRO operation because one LRO operation could have more than one success responses(200, 201, 202) so we narrowed it. And we only need to consider deserializing the final logic response.
  • [newly added] Sub-model relationship between default responses and non-default responses

@MaryGao MaryGao marked this pull request as ready for review July 9, 2024 09:11
@MaryGao MaryGao changed the title Fix the isUnexpected narrowing issue Fix the isUnexpected narrowing issue in modular Jul 9, 2024
MaryGao
MaryGao commented Jul 15, 2024
View reviewed changes
...c-test/test/DeviceRegistry.Management/generated/typespec-ts/src/api/operationStatus/index.ts Outdated
throw createRestError(result);
}

const _result = result as unknown as OperationStatusGet200Response;
Copy link
Member Author

@MaryGao MaryGao Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we only explicitly narrowed the type when necessary.
// the smoke test case will be removed.

@MaryGao MaryGao requested a review from qiaozha July 16, 2024 00:58
@MaryGao MaryGao requested a review from qiaozha July 19, 2024 02:54
@MaryGao MaryGao enabled auto-merge (squash) July 23, 2024 03:37
@MaryGao MaryGao disabled auto-merge July 23, 2024 03:37
@MaryGao MaryGao enabled auto-merge (squash) July 23, 2024 04:04
@MaryGao MaryGao disabled auto-merge July 23, 2024 04:05
@MaryGao MaryGao merged commit 4aff739 into Azure:main Jul 23, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joheredi joheredi joheredi approved these changes

@qiaozha qiaozha qiaozha approved these changes

@sarangan12 sarangan12 Awaiting requested review from sarangan12 sarangan12 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

isUnexpected not working if one model is a subset of another model
5 participants
@MaryGao @joheredi @qiaozha @kazrael2119 @v-jiaodi