-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Update Utf8JsonWriter for null strings, double/single formatting, and consistency. #3143
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.
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
Conversation
<summary>Gets the custom behavior when writing JSON using this instance, which indicates whether to format the output while writing and whether to skip structural JSON validation.</summary> | ||
<value>An object that defines the behavior of this instance for formatting and validation.</value> | ||
<summary>Gets the custom behavior when writing JSON using this instance, which indicates whether to format the output while writing, whether to skip structural JSON validation, and which characters to escape.</summary> | ||
<value>Defines the behavior of this instance of the writer for formatting, validation, and escaping.</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JsonWriterOptions is a struct, not a class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's my understanding that <value>
needs to explain the data type and what it represents. So you could prepend it with "struct instance" for example. Also, as I mentioned in your other PR, in our official docs for "C# objects", it seems that we use "object" to describe a struct instance too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this? It shouldn't start with a verb here.
<value>Defines the behavior of this instance of the writer for formatting, validation, and escaping.</value> | |
<value>The custom behavior of this instance of the writer for formatting, validating, and escaping.</value> |
@@ -3155,6 +3155,8 @@ The property name and value are escaped before writing. | |||
|
|||
The property name and value are escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.ReadOnlySpan{byte})"/> was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressing recent changes around "null:
#3081 (comment)
This PR updated the description around nulls in a few places. We should make sure to capture that:
dotnet/corefx#39344
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.ReadOnlySpan{byte})"/> was called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNull(System.ReadOnlySpan{System.Byte})"> method was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the langword null here (rather than back-ticks)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. Yep, that needs to be fixed. Thanks, @mairaw.
@@ -56,7 +56,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="bufferWriter">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see langword="System.Buffers.IBufferWriter<System.Byte>" />.</summary> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.Buffers.IBufferWriter`1" /> to write the output to.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the build didn't like using
`IBufferWriter<byte>`
So, I decided to use the generic one. Is there a correct way to specify that the T is byte?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above.
@@ -80,7 +80,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="utf8Json">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <paramref name="utf8Json" />.</summary> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.IO.Stream" /> to write the output to.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We normally mention streams with plain language:
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified stream to write the output to.</summary>
Edit: Use Maira's suggestion below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add to Carlos suggestion and use the text from our wiki - https://github.com/dotnet/dotnet-api-docs/wiki/Summary%3A-Constructor
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.IO.Stream" /> to write the output to.</summary> | |
<summary>Initializes a new instance of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> class using the specified stream to write the output to and customization options.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some changes for you to consider.
Let's also wait for @mairaw / @rpetrusha 's review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments for you to consider.
@@ -56,7 +56,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="bufferWriter">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see langword="System.Buffers.IBufferWriter<System.Byte>" />.</summary> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.Buffers.IBufferWriter`1" /> to write the output to.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow GitHub won't let me do a suggestion for this line but it should be something like:
Initializes a new instance of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> class using the specified buffer and customization options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens to me to sometimes, @mairaw. Press Control+G on the comment textbox, or manually add the suggestion text. Here's your suggestion:
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.Buffers.IBufferWriter`1" /> to write the output to.</summary> | |
<summary>Initializes a new instance of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> class using the specified buffer and customization options.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IBufferWriter isn't a buffer though. I would prefer to keep that as is, and also specify that the IBW is where we are "writing the output to".
Any better way to word it to keep both those things?
@@ -80,7 +80,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="utf8Json">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <paramref name="utf8Json" />.</summary> | |||
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.IO.Stream" /> to write the output to.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add to Carlos suggestion and use the text from our wiki - https://github.com/dotnet/dotnet-api-docs/wiki/Summary%3A-Constructor
<summary>Constructs a new <see cref="T:System.Text.Json.Utf8JsonWriter" /> instance with a specified <see cref="T:System.IO.Stream" /> to write the output to.</summary> | |
<summary>Initializes a new instance of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> class using the specified stream to write the output to and customization options.</summary> |
@@ -80,7 +80,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="utf8Json">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" />. By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
@@ -56,7 +56,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="bufferWriter">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding missing dot to @mairaw's suggestion.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" />. By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The options isn't an enum though.
<summary>Gets the custom behavior when writing JSON using this instance, which indicates whether to format the output while writing and whether to skip structural JSON validation.</summary> | ||
<value>An object that defines the behavior of this instance for formatting and validation.</value> | ||
<summary>Gets the custom behavior when writing JSON using this instance, which indicates whether to format the output while writing, whether to skip structural JSON validation, and which characters to escape.</summary> | ||
<value>Defines the behavior of this instance of the writer for formatting, validation, and escaping.</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this? It shouldn't start with a verb here.
<value>Defines the behavior of this instance of the writer for formatting, validation, and escaping.</value> | |
<value>The custom behavior of this instance of the writer for formatting, validating, and escaping.</value> |
@@ -3415,6 +3417,8 @@ The property name and value are escaped before writing. | |||
|
|||
The property name and value are escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.ReadOnlySpan{char})"/> was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.ReadOnlySpan{char})"/> was called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNull(System.ReadOnlySpan{System.Char})"> method was called. |
@@ -3680,6 +3684,8 @@ The property name and value are escaped before writing. | |||
|
|||
The property name and value are escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(string)"/> were called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(string)"/> were called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNull(System.String)"> method was called. |
@@ -3909,6 +3915,8 @@ The property name should already be escaped when the instance of <xref:System.Te | |||
|
|||
The value is escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.Text.Json.JsonEncodedText)"/> was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.Text.Json.JsonEncodedText)"/> was called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNull(System.Text.Json.JsonEncodedText)"> method was called. |
@@ -3947,6 +3955,8 @@ The property name should already be escaped when the instance of <xref:System.Te | |||
|
|||
The value is escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.Text.Json.JsonEncodedText)"/> was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNull(System.Text.Json.JsonEncodedText)"/> was called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNull(System.Text.Json.JsonEncodedText)"> method was called. |
@@ -4185,6 +4195,8 @@ The value is escaped before writing. | |||
|
|||
The value is escaped before writing. | |||
|
|||
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNullValue"/> was called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If <paramref name="value"/> is <see langword="null"/> the JSON null value is written, as if <see cref="WriteNullValue"/> was called. | |
If `value` is `null`, the JSON null value is written as if the <xref:System.Text.Json.Utf8JsonWriter.WriteNullValue"> method was called. |
@@ -821,7 +821,7 @@ The comment value is not escaped before writing. | |||
|
|||
-or- | |||
|
|||
<paramref name="utf8Value" /> contains a comment delimiter (that is, */).</exception> | |||
<paramref name="utf8Value" /> contains a comment delimiter (that is, `*/`).</exception> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Maira's suggestion above, use <c>
if it renders correctly after the build finishes:
<paramref name="utf8Value" /> contains a comment delimiter (that is, `*/`).</exception> | |
<paramref name="utf8Value" /> contains a comment delimiter (that is, <c>*/</c>).</exception> |
I'll leave the suggestions to make it easier to commit them.
@@ -843,7 +843,7 @@ The comment value is not escaped before writing. | |||
<Parameter Name="value" Type="System.ReadOnlySpan<System.Char>" /> | |||
</Parameters> | |||
<Docs> | |||
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within /*..*/.</param> | |||
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within `/*..*/`.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within `/*..*/`.</param> | |
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within <c>/*..*/</c>.</param> |
|
||
-or- | ||
|
||
<paramref name="value" /> contains a comment delimiter (that is, */).</exception> | ||
<paramref name="value" /> contains a comment delimiter (that is, `*/`).</exception> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<paramref name="value" /> contains a comment delimiter (that is, `*/`).</exception> | |
<paramref name="value" /> contains a comment delimiter (that is, <c>*/</c>).</exception> |
@@ -880,7 +880,7 @@ The comment value is not escaped before writing. | |||
<Parameter Name="value" Type="System.String" /> | |||
</Parameters> | |||
<Docs> | |||
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within /*..*/.</param> | |||
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within `/*..*/`.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within `/*..*/`.</param> | |
<param name="value">The UTF-16 encoded value to be written as a UTF-8 transcoded JSON comment within <c>/*..*/</c>.</param> |
@@ -895,7 +895,7 @@ The comment value is not escaped before writing. | |||
|
|||
-or- | |||
|
|||
<paramref name="value" /> contains a comment delimiter (that is, */).</exception> | |||
<paramref name="value" /> contains a comment delimiter (that is, `*/`).</exception> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<paramref name="value" /> contains a comment delimiter (that is, `*/`).</exception> | |
<paramref name="value" /> contains a comment delimiter (that is, <c>*/</c>).</exception> |
@@ -56,7 +56,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="bufferWriter">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding missing dot to @mairaw's suggestion.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" />. By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
@@ -80,7 +80,7 @@ To be able to format the output with indentation and white space OR to skip vali | |||
<Docs> | |||
<param name="utf8Json">The destination for writing JSON text.</param> | |||
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<param name="options">Defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" /> By default, it writes minimized JSON (with no extra whitespace) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
<param name="options">One of the enumeration values that that defines the customized behavior of the <see cref="T:System.Text.Json.Utf8JsonWriter" />. By default, it writes minimized JSON (with no extra white space) and validates that the JSON being written is structurally valid according to the JSON RFC.</param> | |
@@ -383,7 +383,7 @@ The <xref:System.Text.Json.Utf8JsonWriter> will continue to use the original wri | |||
|
|||
## Remarks | |||
|
|||
The <xref:System.Text.Json.Utf8JsonWriter> will continue to use the original writer options but now writes to `bufferWriter` as the new destination. | |||
The <xref:System.Text.Json.Utf8JsonWriter> will continue to use the original writer options but now write to `bufferWriter` as the new destination. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove the "s"? Utf8JsonWriter is still a singular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seemed to make more grammatical sense to my ear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on the fence on this one. @rpetrusha can you help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @jozkee, The Utf8JsonWriter is who's writing. It writes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option would be to add will write
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tense of write is future, so it should be "write" rather than "writes". As Maira points out, it could also be changed to "will write", and that's fine, but it's not necessary.
Anything left to do or is this good to merge? |
@@ -2756,7 +2756,7 @@ The property name should already be escaped when the instance of <xref:System.Te | |||
</remarks> | |||
<exception cref="T:System.InvalidOperationException">The depth of the JSON has exceeded the maximum depth of 1000. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I could also do this in a separate PR
<exception cref="T:System.InvalidOperationException">The depth of the JSON has exceeded the maximum depth of 1000. | |
<exception cref="T:System.InvalidOperationException">The depth of the JSON has exceeded the maximum depth of 1,000. |
I'll merge this and fix the grammar if needed on a separate PR. |
Also updated JsonReaderOptions and JsonWriterOptions to fix some nits.
cc @mairaw, @rpetrusha, @carlossanlop