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

Fix custompage issue when there are two #3389

Merged
merged 6 commits into from
May 12, 2023

Conversation

pshao25
Copy link
Member

@pshao25 pshao25 commented May 11, 2023

Fix #3386

@m-nash
Copy link
Member

m-nash commented May 11, 2023

It appears the current regen shows some odd swapping from ctor to Create which doesn't seem right. https://github.com/Azure/azure-sdk-for-net/pull/36206/files#diff-a076be848338a356eff6ca0db1f64dfb29bd595405264856797b6041c97e2ad2R123

Please send an updated regen PR. Did we manually check this against this branch to see that the two lists now generate as expected?

@pshao25
Copy link
Member Author

pshao25 commented May 12, 2023

It appears the current regen shows some odd swapping from ctor to Create which doesn't seem right. https://github.com/Azure/azure-sdk-for-net/pull/36206/files#diff-a076be848338a356eff6ca0db1f64dfb29bd595405264856797b6041c97e2ad2R123

Please send an updated regen PR. Did we manually check this against this branch to see that the two lists now generate as expected?

This is already updated. We have that change in regen PR is because @JoshLove-msft did this change manually Azure/azure-sdk-for-net#36192 and testcommon doesn't have a ci.yml there so we cannot regen after that manual change. @JoshLove-msft I think the autogenerated code is expected, we shouldn't have manually change them. Is there any reason you did this?

Yes, I checked their entire spec and the generated code is expected. We have test in Pagination-Typespec as well.

@JoshLove-msft
Copy link
Member

JoshLove-msft commented May 12, 2023

The testcommon library is pinned to a specific version of AutoRest so it doesn't pick up new changes. Should we try to unpin it?

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/testcommon/Azure.Graph.Rbac/src/Azure.Graph.Rbac.csproj#L16

@pshao25
Copy link
Member Author

pshao25 commented May 12, 2023

The testcommon library is pinned to a specific version of AutoRest so it doesn't pick up new changes. Should we try to unpin it?

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/testcommon/Azure.Graph.Rbac/src/Azure.Graph.Rbac.csproj#L16

@m-nash Seems you did this change. Any background here? I create an issue here: Azure/azure-sdk-for-net#36235

@m-nash
Copy link
Member

m-nash commented May 12, 2023

The testcommon library is pinned to a specific version of AutoRest so it doesn't pick up new changes. Should we try to unpin it?

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/testcommon/Azure.Graph.Rbac/src/Azure.Graph.Rbac.csproj#L16

We can't unpin it, we would either need to sync the release label that is pinned now and create a fix in a hotfix branch and release from there and update the version, OR we need to move this to manual code, we cannot change generated files manually.

@m-nash
Copy link
Member

m-nash commented May 12, 2023

Commented out the download shared source check until this gets resolved #3392

Copy link
Member

@m-nash m-nash left a comment

Choose a reason for hiding this comment

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

Validated the fix in the content saftey pr

@m-nash m-nash merged commit 8b06121 into Azure:feature/v3 May 12, 2023
@pshao25 pshao25 deleted the customPage3386 branch May 15, 2023 02:45
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.

[ContentSafety] The return type of pageable method is not correct
5 participants