Azure / cadl-ranch

Cadl Scenarios for client generations
https://azure.github.io/cadl-ranch/
MIT License
8 stars 33 forks source link

fix ARM scenario names #714

Closed XiaofeiCao closed 1 month ago

XiaofeiCao commented 3 months ago

Related comment: https://github.com/Azure/cadl-ranch/pull/701#discussion_r1735033126

Shouldn't affect existing generated SDK code. Verified for Java, codegen PR: https://github.com/microsoft/typespec/pull/4717 May affect existing coverage data in dashboard, though should be auto-updated after each SDK's generator CI runs.

Breaking changes:

1. Renamed namespace `Azure.ResourceManager.Models.Resources` to `Azure.ResourceManager.Resources`.
2. Renamed namespace `Azure.ResourceManager.Models.CommonTypes.ManagedIdentity` to `Azure.ResourceManager.CommonProperties`.
3. Renamed folder `common-types/managed-identity` to `common-properties`.
4. Renamed `ManagedIdentityTrackedResources` interface in `common-properties` to `ManagedIdentity`.

Cadl Ranch Contribution Checklist:

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 9ee03d20b27510bed58bc9178672d463e214acdb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ----------------------------- | ----- | | @azure-tools/cadl-ranch-specs | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

XiaofeiCao commented 2 months ago

@iscai-msft I've renamed the namespace according to https://github.com/Azure/cadl-ranch/pull/703#discussion_r1744043677

I removed the extra s, e.g. Resources->Resource, guess it's better to be shown in dashboard.

Let me know your thoughts.

iscai-msft commented 2 months ago

@XiaofeiCao is this PR ready to re-review?

XiaofeiCao commented 2 months ago

@XiaofeiCao is this PR ready to re-review?

Yes, please! Would like a re-review for this PR, since it's required for future ARM test cases.

I added breaking change note in changeset, let me know if it looks good.

XiaofeiCao commented 2 months ago

Thanks @XiaofeiCao this looks great! Can you make sure @timotheeguerin approves first, and that folks in shanghai are aware of the breaking change? After that is done, should be good to merge. Thanks again for all of your changes and all the work you've put into this, it looks great

Yes, shanghai folks are informed of this breaking change. Thanks a lot for your suggestions!

XiaofeiCao commented 1 month ago

Recommend that you tested this in Java (as it seems involve some mockapi change as well).

Yeah, regened and tested: https://github.com/microsoft/typespec/pull/4717