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

[DRAFT] 2.0 upgrade guide working doc #2298

Merged
merged 23 commits into from
Apr 11, 2025
Merged

[DRAFT] 2.0 upgrade guide working doc #2298

merged 23 commits into from
Apr 11, 2025

Conversation

RachitMalik12
Copy link
Contributor

No description provided.

@RachitMalik12
Copy link
Contributor Author

@martincostello Hi Martin! Please refer to this as a work in progress, upgrade guide. We will work on this in the open and keeping adding to it as we have more preview bits added. Please let us know if you have any feedback or questions or if we are missing anything you'd like more info on.
cc: @captainsafia @baywet

@martincostello
Copy link
Contributor

@RachitMalik12 Thanks for providing this. I've made various suggested edits to the current content.

My main feedback so far based on my (still ongoing) move in Swashbuckle would be to have a dedicated section showing how the reference handling has changed with the type checks. For example the need to do things like:

public void ProcessSchema(IOpenApiSchema schema)
{
    if (schema is OpenApiSchema concrete)
    {
        // Do something
    }
    else if (scheme is OpenApiSchemaReference reference)
    {
        // Do something else
    }
}

I'll be sure to leave any additional feedback that might be useful once I've gotten Swashbuckle working with the preview7 (as that's what ASP.NET Core 10 preview 2 is compiled against).

@martincostello
Copy link
Contributor

I've also been tripped up by IOpenApiSchema.Type now being a [Flags] enum and had to fix a few things like this:

- if (schema.Type == JsonSchemaTypes.Array)
+ if (schema.Type is { } type && type.HasFlag(JsonSchemaTypes.Array))

People might miss this and introduce subtle bugs if they just one-to-one map the strings to the values like:

- if (schema.Type == "array")
+ if (schema.Type == JsonSchemaTypes.Array)

Co-authored-by: Martin Costello <[email protected]>
@RachitMalik12 RachitMalik12 requested a review from a team as a code owner April 2, 2025 18:45
RachitMalik12 and others added 6 commits April 2, 2025 11:45
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>

This comment was marked as outdated.

RachitMalik12 and others added 5 commits April 2, 2025 11:53
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Martin Costello <[email protected]>

This comment was marked as outdated.

@MaggieKimani1 MaggieKimani1 linked an issue Apr 3, 2025 that may be closed by this pull request
baywet and others added 2 commits April 9, 2025 12:36
baywet added 2 commits April 9, 2025 15:47
Signed-off-by: Vincent Biret <[email protected]>
Copy link

sonarqubecloud bot commented Apr 9, 2025

@RachitMalik12
Copy link
Contributor Author

Hey @martincostello,
Thanks for the feedback! We have decided to merge this PR and open a new one with a new set of changes so we can keep this guide moving and ensure it is sync'ed with code changes including the one you've listed below and the Flags enum.
We would also be happy to receive any contributions regarding the requested additions to the doc - if you have the time of course :) - no worries if not, the team is doing their best to keep this moving along given the resources we have - I know it has been slow, apologies for the delays.

Also, one concern I have is that the version you are using (preview7) is a few months old and we've made some more updates since then, @captainsafia what is the roadmap for moving ASP.NET to a more later preview? I just worry if some of the issues you encounter during your upgrade are fixed in the later previews, so you don't need to spend the extra time in fixing it.
cc: @baywet

@RachitMalik12 Thanks for providing this. I've made various suggested edits to the current content.

My main feedback so far based on my (still ongoing) move in Swashbuckle would be to have a dedicated section showing how the reference handling has changed with the type checks. For example the need to do things like:

public void ProcessSchema(IOpenApiSchema schema)
{
    if (schema is OpenApiSchema concrete)
    {
        // Do something
    }
    else if (scheme is OpenApiSchemaReference reference)
    {
        // Do something else
    }
}

I'll be sure to leave any additional feedback that might be useful once I've gotten Swashbuckle working with the preview7 (as that's what ASP.NET Core 10 preview 2 is compiled against).

@martincostello
Copy link
Contributor

ASP.NET Core 10 Preview 3 shipped yesterday, so I'm now up to preview.11. Due to breaking changes in subsequent previews there are constraints on how far forwards I can jump in my own code before I hit a "compatibility wall".

@baywet
Copy link
Member

baywet commented Apr 11, 2025

ASP.NET Core 10 Preview 3 shipped yesterday, so I'm now up to preview.11. Due to breaking changes in subsequent previews there are constraints on how far forwards I can jump in my own code before I hit a "compatibility wall".

yes the goal is that once the initial file is merged we can:

  1. enforce that any new breaking change PR also contains a modification to this file
  2. work our way out of the "documentation debt" in multiple separate PRs (and of course, external contributions are always welcome)

@baywet baywet merged commit 2b159ad into main Apr 11, 2025
15 checks passed
@baywet baywet deleted the rm/upgrade-guide branch April 11, 2025 16:27
@baywet
Copy link
Member

baywet commented Apr 11, 2025

@martincostello, now that this is merged, would you mind going ahead and proposing your changes as a PR please?

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.

Upgrade Guide for v2
4 participants