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

DOT NOT MERGE - SMOKE TEST #2009

Closed
wants to merge 1 commit into from
Closed

Conversation

andrueastman
Copy link
Member

@andrueastman andrueastman commented Jul 7, 2023

This pull request was automatically created by Azure Pipelines. Important Check for unexpected deletions or changes in this PR.

Microsoft Reviewers: Open in CodeFlow
Microsoft Reviewers: codeflow:open?pullrequest=https://github.com/microsoftgraph/msgraph-sdk-dotnet/pull/2009&drop=dogfoodAlpha

@andrueastman andrueastman requested a review from a team as a code owner July 7, 2023 21:33
@baywet baywet changed the title Generated models and request builders DOT NOT MERGE - SMOKE TEST Jul 7, 2023
@baywet
Copy link
Member

baywet commented Jul 7, 2023

@andrueastman here is what still looks odd at this point:

  • src/Microsoft.Graph/Generated/Models/IdentityGovernance/ValueTypeObject.cs EnumObject and StringObject members are now Enum and String, which is technically correct but a breaking change
  • that's it! everything else looks good.

Could you quickly whip up a PR for kiota to address that last remaining regression so we can release kiota 1.4.0 please?

@andrueastman
Copy link
Member Author

@baywet
As this is a relatively new type in the code introduced about a week ago via #1993, I'd argue we label this as a fix and go ahead with the correct member names and have it clearly labeled in the changelog.
As these members should never really have ended up with the Object suffix, microsoft/kiota#2874 together with making the CodeEnumOption ended up serendipitously fixing a bug were enum options would be replaced if and only if the name of the Enum itself was replaced. This is because, the ReplaceReservedModelTypes would pass a different set of exceptions types and thus the nested enum option would be processed simply because the parent was being renamed(which is no longer the case as these are processed independently now).

@baywet
Copy link
Member

baywet commented Jul 10, 2023

@andrueastman thanks for providing the context here. Yes let's label that a fix and move on so we don't end up carrying an odd compatibility patch forever. Thanks. Closing.

@baywet baywet closed this Jul 10, 2023
@baywet baywet deleted the kiota/v1.0/pipelinebuild/119551 branch July 10, 2023 11:09
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.

2 participants