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

arm, remove special handling for ARM types #2749

Merged
merged 4 commits into from
May 11, 2024

Conversation

XiaofeiCao
Copy link
Contributor

typespec-azure-resource-manager has fixed the naming issue: Azure/typespec-azure#762
Our special handling for Resource, ProxyResource and ArmResource is no longer needed.

// skip models under "Azure.ResourceManager"
if (this.isArm() && schema.language.default?.namespace?.startsWith("Azure.ResourceManager")) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weidongxu-microsoft
Copy link
Member

ArmResource is deprecated but not removed. Do we still need this till it get removed?

@XiaofeiCao
Copy link
Contributor Author

XiaofeiCao commented May 10, 2024

ArmResource is deprecated but not removed. Do we still need this till it get removed?

I searched current specs repo, seems (Arm)Resource is only used in template parameters as : https://github.com/search?q=repo%3AAzure%2Fazure-rest-api-specs+%22ArmResource%22&type=code&p=1
I added back one ArmResource in our test, seems ok.

(Arm)Resource is used as base model for all Tracked/Proxy/ExtensionResource(Base). I guess service is not allowed to let its models directly extend ArmResource, otherwise it'll break JS and Python..

@XiaofeiCao
Copy link
Contributor Author

Offline sync with Chenjie, service will not use ArmResource for green field. For brownfields, there exists cases where model directly extends Resource, and will be converted to Resource in tsp, still no ArmResource. And existing ArmResource will be fixed to Resource.

@XiaofeiCao XiaofeiCao marked this pull request as ready for review May 10, 2024 08:00
@XiaofeiCao XiaofeiCao merged commit e612740 into Azure:main May 11, 2024
3 checks passed
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