Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Revert #80698 and obsolete JsonSerializerOptions.AddContext #84022

Merged
merged 2 commits into from
Mar 30, 2023
Merged

Revert #80698 and obsolete JsonSerializerOptions.AddContext #84022

merged 2 commits into from
Mar 30, 2023

Conversation

eiriktsarpalis
Copy link
Member

Fix #83280.

@eiriktsarpalis eiriktsarpalis self-assigned this Mar 28, 2023
@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone Mar 28, 2023
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Mar 28, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #83280.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json, new-api-needs-documentation
Milestone: 8.0.0

@eiriktsarpalis eiriktsarpalis added breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet and removed new-api-needs-documentation labels Mar 28, 2023
@ghost
Copy link

ghost commented Mar 28, 2023

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

eerhardt
eerhardt approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM.

Can we hold off on merging this until dotnet/aspnetcore#47450 is merged to move ASP.NET off AddContext?

@build-analysis build-analysis bot mentioned this pull request Mar 28, 2023
Closed
@eiriktsarpalis eiriktsarpalis added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 28, 2023
@eiriktsarpalis
Copy link
Member Author

Holding until aspnetcore remove their dependency on AddContext.

This was referenced Mar 29, 2023
Merged
Closed
@eerhardt
Copy link
Member

@eiriktsarpalis - FYI dotnet/aspnetcore#47450 has been merged, so we no longer use AddContext in ASP.NET.

@eiriktsarpalis eiriktsarpalis removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 30, 2023
@eiriktsarpalis
Copy link
Member Author

Test failures are unrelated, merging away.

@eiriktsarpalis eiriktsarpalis merged commit e43ddbf into dotnet:main Mar 30, 2023
@eiriktsarpalis eiriktsarpalis deleted the deprecate-addcontext branch March 30, 2023 11:40
@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2023
@dotnet dotnet unlocked this conversation Oct 16, 2023
@eiriktsarpalis eiriktsarpalis removed breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Oct 16, 2023
@eiriktsarpalis
Copy link
Member Author

Removing the breaking-change labels since they're not actually needed. This PR reverts an initial breaking change that was made to it and the method retains its original behavior only that it has now been obsoleted.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 15, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@eerhardt eerhardt eerhardt approved these changes

@layomia layomia layomia approved these changes

@jeffhandley jeffhandley Awaiting requested review from jeffhandley jeffhandley is a code owner

@krwq krwq Awaiting requested review from krwq

Assignees

@eiriktsarpalis eiriktsarpalis

Labels
area-System.Text.Json
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

Obsolete JsonSerializerOptions.AddContext
3 participants
@eiriktsarpalis @eerhardt @layomia