Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix Utf8JsonWriter xml docs#39092

Merged
safern merged 8 commits intodotnet:masterfrom
safern:FixUtf8WritePropertiesDocs
Jul 16, 2019
Merged

Fix Utf8JsonWriter xml docs#39092
safern merged 8 commits intodotnet:masterfrom
safern:FixUtf8WritePropertiesDocs

Conversation

@safern
Copy link
Copy Markdown
Member

@safern safern commented Jul 1, 2019

@safern
Copy link
Copy Markdown
Member Author

safern commented Jul 3, 2019

@stephentoub looks good?

@ahsonkhan ahsonkhan added area-System.Text.Json documentation Documentation bug or enhancement, does not impact product or test code labels Jul 4, 2019
bartonjs
bartonjs reviewed Jul 5, 2019
View reviewed changes
bartonjs
bartonjs reviewed Jul 5, 2019
View reviewed changes
@ericstj
Copy link
Copy Markdown
Member

ericstj commented Jul 15, 2019

@safern were you planning on finishing this one up?

@safern
Copy link
Copy Markdown
Member Author

safern commented Jul 15, 2019

@safern were you planning on finishing this one up?

Yep, I will update it soon.

ericstj
ericstj approved these changes Jul 16, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

I found one duplicate remark, and it needs rebasing, but other than that it looks ok.

ahsonkhan
ahsonkhan approved these changes Jul 16, 2019
View reviewed changes
Copy link
Copy Markdown

@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.

Some leftover feedback. It was difficult to review this because of the remarks/exception tags re-ordering so maybe something got missed, but it would be low risk :)

@safern safern force-pushed the FixUtf8WritePropertiesDocs branch from 4ea8e32 to 68ca05a Compare July 16, 2019 18:53
@ahsonkhan ahsonkhan added this to the 3.0 milestone Jul 16, 2019
@safern
Copy link
Copy Markdown
Member Author

safern commented Jul 16, 2019

Merging as this doesn't affect tests.

@safern safern merged commit 7b90f41 into dotnet:master Jul 16, 2019
@safern safern deleted the FixUtf8WritePropertiesDocs branch July 16, 2019 21:15
@ericstj
Copy link
Copy Markdown
Member

ericstj commented Jul 16, 2019

Thanks @safern

@ahsonkhan
Copy link
Copy Markdown

ahsonkhan commented Jul 18, 2019

Since these were updates to existing xml comments (and not on new APIs/missing docs), this will need manual fix up within the docs repo, based on @safern's changes/word-smithing here.

For example: https://github.com/dotnet/dotnet-api-docs/blob/4b6849c3dbea830643f6a0023be4fb93ddfc3b29/xml/System.Text.Json/Utf8JsonWriter.xml#L2487

cc @carlossanlop, @@rpetrusha, @mairaw

@ahsonkhan ahsonkhan mentioned this pull request Jul 18, 2019
Merged
@safern safern mentioned this pull request Aug 9, 2019
Merged
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@safern
Fix Utf8JsonWriter xml docs (dotnet/corefx#39092)
daae51d 
* Fix Utf8JsonWriter xml docs

* PR Feedback

* PR Feedback

* PR Feedback put back remarks for ROS/string overrides

* Remove duplicate remarks

* PR Feedback

* PR Feedback, i.e. -> that is


Commit migrated from dotnet/corefx@7b90f41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

6 more reviewers

@carlossanlop carlossanlop carlossanlop left review comments

@stephentoub stephentoub stephentoub left review comments

@ahsonkhan ahsonkhan ahsonkhan approved these changes

@bartonjs bartonjs bartonjs left review comments

@mairaw mairaw mairaw left review comments

@ericstj ericstj ericstj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Text.Json documentation Documentation bug or enhancement, does not impact product or test code

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

The Utf8JsonWriter API xml documentation could be improved

7 participants

@safern @ericstj @ahsonkhan @carlossanlop @stephentoub @bartonjs @mairaw