Skip to content

Automatically port System.Text.Json.Serialization.JsonConverter #2889

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 6, 2019
Merged

Automatically port System.Text.Json.Serialization.JsonConverter #2889

merged 5 commits into from
Aug 6, 2019

Conversation

carlossanlop
Copy link
Contributor

New batch.

@carlossanlop carlossanlop added new-content Indicates PRs that contain new articles waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews 🏁 Release: .NET Core 3.0 :checkered_flag: Release: .NET Core 3.0 labels Jul 31, 2019
@carlossanlop carlossanlop added this to the July 2019 milestone Jul 31, 2019
@carlossanlop carlossanlop self-assigned this Jul 31, 2019
@mairaw mairaw modified the milestones: July 2019, August 2019 Jul 31, 2019
jozkee
jozkee requested changes Aug 1, 2019
View reviewed changes
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

Left a few comments.

jozkee
jozkee requested changes Aug 1, 2019
View reviewed changes
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

One more suggestion.

rpetrusha
rpetrusha suggested changes Aug 1, 2019
View reviewed changes
Copy link

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

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

@jozkee has made a number of suggestions, @carlossanlop, one of which I've edited.

@carlossanlop @jozkee
suggestions by rpetrusha and Jozkee
62ad193 
Co-Authored-By: David Cantu <jozkyy@gmail.com>
@carlossanlop carlossanlop requested a review from rpetrusha August 1, 2019 17:21
@carlossanlop carlossanlop added changes-addressed Indicates PRs that had all comments addressed and are awaiting for new review and removed waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews labels Aug 1, 2019
jozkee
jozkee requested changes Aug 1, 2019
View reviewed changes
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

Sorry for the typo in my previous suggestion.

@carlossanlop @jozkee
suggestion by Jozkee
c133c12 
Co-Authored-By: David Cantu <jozkyy@gmail.com>
ahsonkhan
ahsonkhan reviewed Aug 2, 2019
View reviewed changes
ahsonkhan
ahsonkhan reviewed Aug 2, 2019
View reviewed changes
@ahsonkhan ahsonkhan mentioned this pull request Aug 2, 2019
Merged
@carlossanlop @ahsonkhan
suggestions by ahsonkhan
9ecb192 
Co-Authored-By: Ahson Khan <ahkha@microsoft.com>
ahsonkhan
ahsonkhan approved these changes Aug 2, 2019
View reviewed changes
Copy link
Contributor

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM.

@carlossanlop @ahsonkhan
suggestion by ahsonkhan
f88c1c0 
Co-Authored-By: Ahson Khan <ahkha@microsoft.com>
@carlossanlop
Copy link
Contributor Author

@rpetrusha the latest suggestion has been addressed. If it looks good, can we get it merged if the build finishes successfully?

@carlossanlop carlossanlop added verify-build-before-merge and removed changes-addressed Indicates PRs that had all comments addressed and are awaiting for new review labels Aug 6, 2019
@carlossanlop
Copy link
Contributor Author

@rpetrusha the build passed, I think this one's good to merge.

@rpetrusha
Copy link

It is good to merge, @carlossanlop. I'll do that now.

@rpetrusha rpetrusha merged commit a6a33de into dotnet:master Aug 6, 2019
@carlossanlop carlossanlop deleted the JsonConverter branch August 6, 2019 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveharter steveharter Awaiting requested review from steveharter

@mairaw mairaw Awaiting requested review from mairaw

@jozkee jozkee Awaiting requested review from jozkee

2 more reviewers

@ahsonkhan ahsonkhan ahsonkhan approved these changes

@rpetrusha rpetrusha rpetrusha approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@carlossanlop carlossanlop

Labels
🏁 Release: .NET Core 3.0 :checkered_flag: Release: .NET Core 3.0 new-content Indicates PRs that contain new articles
Projects
None yet
Milestone
August 2019
Development

Successfully merging this pull request may close these issues.
5 participants
@carlossanlop @rpetrusha @ahsonkhan @jozkee @mairaw