Skip to content

Revert #80698 and obsolete JsonSerializerOptions.AddContext#84022

Merged
eiriktsarpalis merged 2 commits intodotnet:mainfrom
eiriktsarpalis:deprecate-addcontext
Mar 30, 2023
Merged

Revert #80698 and obsolete JsonSerializerOptions.AddContext#84022
eiriktsarpalis merged 2 commits intodotnet:mainfrom
eiriktsarpalis:deprecate-addcontext

Conversation

@eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Mar 28, 2023

Fix #83280.

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

ghost commented Mar 28, 2023

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 previous release. 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?

layomia
layomia approved these changes Mar 28, 2023
View reviewed changes
@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

eiriktsarpalis commented Mar 28, 2023

Holding until aspnetcore remove their dependency on AddContext.

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

eerhardt commented Mar 29, 2023

@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

eiriktsarpalis commented Mar 30, 2023

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 previous release. 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

eiriktsarpalis commented Oct 16, 2023

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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@eerhardt eerhardt eerhardt approved these changes

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

@krwq krwq Awaiting requested review from krwq

+1 more reviewer

@layomia layomia layomia approved these changes

Reviewers whose approvals may not affect merge requirements

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