Skip to content

Address additional feedback from number-handling PRs - #42505

Merged
layomia merged 2 commits into
dotnet:masterfrom
layomia:number_handling_feedback
Sep 24, 2020
Merged

Address additional feedback from number-handling PRs#42505
layomia merged 2 commits into
dotnet:masterfrom
layomia:number_handling_feedback

Conversation

@layomia

@layomialayomia commented Sep 20, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Follow up for #39363 & #39716.

FYI @ahsonkhan

@layomialayomia added this to the 6.0.0 milestone Sep 20, 2020
@layomialayomia self-assigned this Sep 20, 2020
@layomialayomia changed the title Address further feedback from number-handling PRsAddress additional feedback from number-handling PRsSep 20, 2020
ahsonkhan
ahsonkhan approved these changes Sep 20, 2020
View reviewed changes
Comment threadsrc/libraries/System.Text.Json/src/System.Text.Json.csproj Outdated
Comment threadsrc/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs Outdated
Comment thread....Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs Outdated
Comment threadsrc/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonNumberHandling.cs Outdated
Comment threadsrc/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonNumberHandling.cs Outdated
Comment threadsrc/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs Outdated
@steveharter

steveharter commented Sep 22, 2020

Copy link
Copy Markdown
Contributor

@layomia note there was a STJ failure in (Libraries Test Run release mono Windows_NT x64 Debug. Appeared the tests didn't run or finish since I didn't see any output. I'll try to restart.

@layomia
layomiaforce-pushed the number_handling_feedback branch from 37372ed to 82f1225CompareSeptember 23, 2020 19:22
@layomia

layomia commented Sep 24, 2020

Copy link
Copy Markdown
ContributorAuthor

note there was a STJ failure in (Libraries Test Run release mono Windows_NT x64 Debug

Thanks. It passed in a subsequent run. I don't think any issue there is due to this PR. I'll watch out for failures in other PRs.

Test failures appear unrelated - 3 timeouts:

runtime (Build Browser wasm Release AllSubsets_Mono) Cancelled after 120m

runtime (Libraries Test Run release coreclr Linux x64 Debug) Cancelled after 150m

runtime (Libraries Test Run release mono Linux x64 Debug) Cancelled after 150m

@layomia
layomia merged commit 54fb9d0 into dotnet:masterSep 24, 2020
@layomia
layomia deleted the number_handling_feedback branch September 24, 2020 18:14
@ghostghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jozkeejozkeeAwaiting requested review from jozkee
2 more reviewers
@ahsonkhanahsonkhanahsonkhan approved these changes
@steveharterstevehartersteveharter approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@layomialayomia

Labels

area-System.Text.Json

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@layomia@steveharter@ahsonkhan