Automatically port System.Text.Json.Utf8JsonReader #2890
Conversation
carlossanlop
added
new-content
carlossanlop
requested review from
adityamandaleeka,
ahsonkhan,
jozkee,
rpetrusha and
steveharter
|
@rpetrusha @jozkee I addressed all the comments and I added a few missing values and params in two additional commits. |
carlossanlop
added
changes-addressed
carlossanlop
requested review from
mairaw
and removed request for
adityamandaleeka
rpetrusha
left a comment
rpetrusha
left a comment
There was a problem hiding this comment.
I've made changes directly in the document, @carlossanlop, so please review them. In addition to reviewing content from this PR, I've also tried to disambiguate overloads.
rpetrusha
added
verify-build-before-merge
and removed
changes-addressed
Co-Authored-By: David Cantu <[email protected]>
|
Thanks @rpetrusha, your last commit looks good and the build passed. I think it's ready to merge. |
|
There are some broken links in the build report. I’ll fix it soon when I connect to my laptop. |
| <returns> | ||
| <see langword="true" /> if the entire UTF-8 encoded token value can be successfully parsed to a <see cref="T:System.DateTimeOffset" /> value; otherwise, <see langword="false" />.</returns> | ||
| <summary>Tries to parse the current JSON token value from the source as a <see cref="T:System.DateTimeOffset" /> and returns a value that indicates whether the operation succeeded.</summary> | ||
| <returns><see langword="true" /> if the entire UTF-8 encoded token value can be successfully parsed to a <see cref="T:System.DateTimeOffset" /> value; otherwise, <see langword="false" />.</returns> |
There was a problem hiding this comment.
I think next time I run CI, it'll break the line again. I think when the value starts with a tag, it automatically will break the line. If you're not doing this manually, then it's fine.
mairaw
left a comment
mairaw
left a comment
There was a problem hiding this comment.
LGTM. I've made some minor changes including fixing the broken links. Left some minor comments as well. Please take a look.
| ## Remarks | ||
| If the look-up text is invalid or incomplete UTF-16 text (for example, it has unpaired surrogates), the method returns `false`, |
There was a problem hiding this comment.
nit: since the remarks are the same for all three overloads, we could move this to a remarks inside a MemberGroup tag.
| <param name="value">If the method succeeds, contains the decoded binary representation of the base 64 text.</param> | ||
| <summary>Tries to parse the current JSON token value from the source and decodes the base 64 encoded JSON string as a byte array.</summary> | ||
| <returns><see langword="true" /> if the entire token value is encoded as valid base 64 text and can be successfully decoded to bytes; <see langword="false" /> otherwise.</returns> | ||
| <param name="value">When this method returns, contains the decoded binary representation of the base 64 text.</param> |
There was a problem hiding this comment.
Is it base 64 or base64 text? @rpetrusha do you know? searching seems to bring it together more often.
| <returns>To be added.</returns> | ||
| <remarks>To be added.</remarks> | ||
| <param name="utf8Text">The UTF-8 encoded text to compare against.</param> | ||
| <summary>Compares the UTF-8 encoded text to the unescaped JSON token value in the source and returns a value that indicates whether they match.</summary> |
There was a problem hiding this comment.
Two overloads have the same description. Wanna try to distinguish them?
|
Thanks for the additional comments, @mairaw. I've incorporated them. The builds look good, so I'm going to merge this PR, @carlossanlop. |
4 participants
New batch.