Skip to content

.NET number values such as positive and negative infinity cannot be written as valid JSON. - #62690

Merged
layomia merged 15 commits into
dotnet:mainfrom
deeprobin:issue-48926
Jan 10, 2022
Merged

.NET number values such as positive and negative infinity cannot be written as valid JSON.#62690
layomia merged 15 commits into
dotnet:mainfrom
deeprobin:issue-48926

Conversation

@deeprobin

@deeprobindeeprobin commented Dec 12, 2021

Copy link
Copy Markdown
Contributor

@ghostghost added community-contribution Indicates that the PR has been added by a community member area-System.Text.Json labels Dec 12, 2021
@ghost

ghost commented Dec 12, 2021

Copy link
Copy Markdown

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

Issue Details

See issue #48926.

/cc @layomia
/cc @GrabYourPitchforks

Author:deeprobin
Assignees:-
Labels:

area-System.Text.Json, community-contribution

Milestone:-

layomia
layomia reviewed Dec 13, 2021
View reviewed changes

@layomialayomia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR should also update the exception message as suggested in #48926 (comment).

Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs Outdated
@deeprobin@layomia
Move doc addition in remark block
16c9d61
Co-authored-by: Layomi Akinrinade <layomia@gmail.com>
layomia
layomia reviewed Dec 13, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs Outdated
deeprobin
deeprobin commented Dec 13, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/src/Resources/Strings.resx
layomia
layomia reviewed Dec 13, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/src/Resources/Strings.resx Outdated
@deeprobin@layomia
Update exception message
a47dc15
Co-authored-by: Layomi Akinrinade <layomia@gmail.com>
krwq
krwq reviewed Dec 14, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs Outdated
@krwqkrwq mentioned this pull request Dec 14, 2021
Merged
deeprobinand others added 2 commits December 14, 2021 18:22
@deeprobin@krwq
Usage of a more insightful message
2d20222
Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>
@deeprobin
Usage of more insightful remark
81a6410
@deeprobin
deeprobin requested a review from krwqDecember 14, 2021 17:25
GrabYourPitchforks
GrabYourPitchforks reviewed Dec 14, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs Outdated
layomia
layomia reviewed Dec 20, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs Outdated
layomia
layomia reviewed Dec 20, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/src/Resources/Strings.resx
layomia
layomia reviewed Dec 20, 2021
View reviewed changes

@layomialayomia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couple more comments, then LGTM.

deeprobinand others added 2 commits December 20, 2021 23:11
@deeprobin@layomia
Change wording
946186b
Co-authored-by: Layomi Akinrinade <layomia@gmail.com>
@deeprobin
Change wording
421398d
layomia
layomia reviewed Dec 22, 2021
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/Common/JsonNumberHandling.cs
@runfoapprunfoappBot mentioned this pull request Dec 22, 2021
Closed
@deeprobin

deeprobin commented Jan 3, 2022

Copy link
Copy Markdown
ContributorAuthor

Couple more comments, then LGTM.

dotnet/dotnet-api-docs#7488 was merged, so I think this could be merged as well

layomia
layomia approved these changes Jan 10, 2022
View reviewed changes

@layomialayomia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @deeprobin!

@layomia
layomia merged commit 7242690 into dotnet:mainJan 10, 2022
@ghostghost locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@krwqkrwqAwaiting requested review from krwq
@GrabYourPitchforksGrabYourPitchforksAwaiting requested review from GrabYourPitchforks
1 more reviewer
@layomialayomialayomia approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@layomialayomia

Labels

area-System.Text.Jsoncommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@deeprobin@krwq@GrabYourPitchforks@layomia