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

Made delegatedIdentityClientId optional CustomerManagedKeyEncryption to align with the common types definition #495

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

abatishchev
Copy link
Contributor

@abatishchev abatishchev commented Mar 22, 2024

Resolves #428.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 22, 2024

All changed packages have been documented.

@abatishchev abatishchev changed the title Made delegatedIdentityClientId a nullable Guid in customer-managed-keys.tsp Made delegatedIdentityClientId a nullable Guid in CustomerManagedKeyEncryption Mar 22, 2024
Copy link
Member

@timotheeguerin timotheeguerin left a comment

Choose a reason for hiding this comment

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

I think this is good to change now as tt should not be breaking the provider hub controller generation as long as the type was generated as a string. With the new change to allow to generate as Guid now the property would be shown differently.

@markcowl let me know if you agree on that

@azure-sdk
Copy link
Collaborator

@abatishchev abatishchev force-pushed the alexbat/delegatedIdentityClientId-1 branch from ab3bb72 to c97ea2d Compare March 22, 2024 03:47
@abatishchev abatishchev changed the title Made delegatedIdentityClientId a nullable Guid in CustomerManagedKeyEncryption Made delegatedIdentityClientId optional CustomerManagedKeyEncryption to align with the common types definition Mar 22, 2024
@abatishchev
Copy link
Contributor Author

@timotheeguerin would I need to submit one more PR to update the pr/providerhub branch?

@timotheeguerin
Copy link
Member

@timotheeguerin would I need to submit one more PR to update the pr/providerhub branch?

in what way? this one + your other to allow Guid generation should be good, and next release everthing works

@abatishchev
Copy link
Contributor Author

Sounds good!

The build fails because I crafted the change log file manually. Will fix it on Wednesday (once back home, right now I have only my phone) until you'd have a moment and update my branch with a correct file (idk why exactly it didn't like the one I made).

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

This looks good - you will need to regenerate the docs by running a repo-wide build (pnpm build), or you can run the task froo regenerating docs directly for just the ARM library (cd packages/typespec-azure-resource-manager; pnpm regen-docs)

@abatishchev abatishchev force-pushed the alexbat/delegatedIdentityClientId-1 branch 2 times, most recently from 10028c0 to 07b93b4 Compare March 27, 2024 05:09
@abatishchev abatishchev force-pushed the alexbat/delegatedIdentityClientId-1 branch from 8c55d8f to 09d4019 Compare March 27, 2024 16:03
@abatishchev
Copy link
Contributor Author

@markcowl done. and the build is green.

@abatishchev abatishchev enabled auto-merge (squash) March 27, 2024 16:03
@abatishchev abatishchev merged commit 34d1d5b into main Mar 27, 2024
14 checks passed
@abatishchev abatishchev deleted the alexbat/delegatedIdentityClientId-1 branch March 27, 2024 16:35
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.

[Bug] TypeSpec for CustomerManagedKeyEncryption in Common Type v5 mismatches with corresponding Swagger spec
4 participants