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

tsp, handle spread behavior change from TCGC #2934

Merged
merged 9 commits into from
Aug 28, 2024

Conversation

Comment on lines 1319 to 1326
// Implicit body parameter would have usage flag: UsageFlags.Spread, for this case we need to do body parameter flatten
const bodyParameterFlatten = sdkType.kind === "model" && sdkType.usage & UsageFlags.Spread && !this.isArm();
// Explicit body parameter @body or @bodyRoot would result to the existance of rawHttpOperation.parameters.body.property
// Implicit body parameter would result to rawHttpOperation.parameters.body.property be undefined
// see https://typespec.io/docs/libraries/http/cheat-sheet#data-types
const bodyParameterFlatten =
sdkType.kind === "model" && !rawHttpOperation.parameters.body?.property && !this.isArm();
Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Aug 26, 2024

Choose a reason for hiding this comment

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

flatten or not should not be based on the model, it should be based on the info of the client method (which is yet to be included in the migration).

@weidongxu-microsoft
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@weidongxu-microsoft
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@weidongxu-microsoft weidongxu-microsoft marked this pull request as ready for review August 27, 2024 03:58
@weidongxu-microsoft weidongxu-microsoft changed the title tsp, try fix spread tsp, handle spread behavior change from TCGC Aug 27, 2024
Comment on lines -1334 to -1338
if (operationIsJsonMergePatch(sdkHttpOperation)) {
// skip model flatten, if "application/merge-patch+json"
schema.language.default.name = pascalCase(op.language.default.name) + "PatchRequest";
return;
}
Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Aug 27, 2024

Choose a reason for hiding this comment

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

and

-      schema.language.default.name = pascalCase(op.language.default.name) + "Request";

These change does not relate to the issue.

But we should use name from TCGC in almost all cases.

This is the cause of the diff of generated code in this PR.

@weidongxu-microsoft
Copy link
Member Author

I will merge the PR after Haoling done the release and sync of 0.20.0

@haolingdong-msft
Copy link
Member

publishing to npm, I will notify here once I triggered sync sdk pipeline on main so that you can merge the pr.

@haolingdong-msft
Copy link
Member

Hi @weidongxu-microsoft, I have triggered sync sdk pipeline on main branch.

@weidongxu-microsoft weidongxu-microsoft merged commit 8fca913 into main Aug 28, 2024
6 checks passed
@weidongxu-microsoft weidongxu-microsoft deleted the tsp_try-fix-spread branch August 28, 2024 09:28
weidongxu-microsoft added a commit to weidongxu-microsoft/typespec that referenced this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants