Skip to content

System.Text.Json based formatters #8362

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 6 commits into from
Mar 21, 2019
Merged

System.Text.Json based formatters #8362

merged 6 commits into from
Mar 21, 2019

Conversation

@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Mar 9, 2019 •
edited
Loading

Fixes #7256

@pranavkm pranavkm requested a review from rynowak March 9, 2019 01:56
davidfowl
davidfowl reviewed Mar 9, 2019
View reviewed changes
davidfowl
davidfowl reviewed Mar 10, 2019
View reviewed changes
@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 10, 2019
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 12, 2019
View reviewed changes
jkotalik
jkotalik reviewed Mar 12, 2019
View reviewed changes
@pranavkm pranavkm mentioned this pull request Mar 13, 2019
Closed
@pranavkm pranavkm force-pushed the prkrishn/formatters branch 4 times, most recently from c0bd648 to 4fe623a Compare March 14, 2019 04:57
@pranavkm pranavkm marked this pull request as ready for review March 14, 2019 04:57
@pranavkm
Copy link
Contributor Author

pranavkm commented Mar 14, 2019

🆙 📅

ahsonkhan
ahsonkhan reviewed Mar 15, 2019
View reviewed changes
rynowak
rynowak reviewed Mar 19, 2019
View reviewed changes
ahsonkhan
ahsonkhan reviewed Mar 20, 2019
View reviewed changes
src/Mvc/Mvc.Core/src/Formatters/TranscodingWriteStream.cs

if (buffer.Length - offset < count)
{
throw new ArgumentOutOfRangeException(nameof(count));
Copy link
Contributor

@ahsonkhan ahsonkhan Mar 20, 2019

Choose a reason for hiding this comment

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

nit: Why did you decide on throwing with count? What if offset > buffer.Length and count = 0?

@pranavkm pranavkm force-pushed the prkrishn/formatters branch from bdc60bf to 377f408 Compare March 20, 2019 23:23
@pranavkm pranavkm force-pushed the prkrishn/formatters branch from 377f408 to 9cf27be Compare March 21, 2019 16:50
@pranavkm pranavkm merged commit f5ff181 into master Mar 21, 2019
@pranavkm pranavkm deleted the prkrishn/formatters branch March 21, 2019 20:45
@jozkee jozkee mentioned this pull request Mar 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidfowl davidfowl davidfowl left review comments

@JamesNK JamesNK JamesNK left review comments

@blowdart blowdart blowdart left review comments

@rynowak rynowak rynowak approved these changes

@GrabYourPitchforks GrabYourPitchforks GrabYourPitchforks left review comments

@stephentoub stephentoub stephentoub left review comments

+2 more reviewers

@ahsonkhan ahsonkhan ahsonkhan left review comments

@jkotalik jkotalik jkotalik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.