Document various isolated System.Text.Json elements #2910
Conversation
carlossanlop
added
new-content
carlossanlop
requested review from
ahsonkhan,
jozkee,
mairaw,
rpetrusha and
steveharter
mairaw
left a comment
mairaw
left a comment
There was a problem hiding this comment.
LGTM. Made a few minor changes. See what you think!
|
The project should also be August now too. We'll soon close the July one. |
|
Thanks @mairaw, your changes look good. The build passed, I think it's good to merge. |
carlossanlop
added
🏁 Release: .NET Core 3.0
| <summary>To be added.</summary> | ||
| <returns>To be added.</returns> | ||
| <param name="typeToConvert">The type to be checked.</param> | ||
| <summary>Determines whether the specified type can be converted to an enum.</summary> |
There was a problem hiding this comment.
enum -> enumeration
| <summary>Determines whether the specified type can be converted to an enum.</summary> | |
| <summary>Determines whether the specified type can be converted to an enumeration.</summary> |
| <param name="typeToConvert">To be added.</param> | ||
| <summary>To be added.</summary> | ||
| <returns>To be added.</returns> | ||
| <param name="typeToConvert">The type to be checked.</param> |
There was a problem hiding this comment.
I think we should follow the inheritdoc comment and change this to whatever we land on for JsonConverter base type (from #2889 (comment)).
I prefer:
| <param name="typeToConvert">The type to be checked.</param> | |
| <param name="typeToConvert">The type of the object to check whether it can be converted by this converter instance.</param> |
| <returns>To be added.</returns> | ||
| <param name="typeToConvert">The type to be checked.</param> | ||
| <summary>Determines whether the specified type can be converted to an enum.</summary> | ||
| <returns><see langword="true" /> if the type can be converted; otherwise, <see langword="false" />.</returns> |
There was a problem hiding this comment.
Similarly here (from #2889 (comment)):
if the type can be converted; otherwise, .
cc @jozkee
| <returns>To be added.</returns> | ||
| <param name="typeToConvert">The type to convert.</param> | ||
| <summary>Create a converter for the specified type.</summary> | ||
| <returns>An instance of <see cref="T:System.Text.Json.Serialization.JsonConverter"/> of type T, where T is compatible with <paramref name="typeToConvert"/>.</returns> |
There was a problem hiding this comment.
Is that how we generally refer to generics? Why not link to JsonConverter<T> rather than "of type T"?
Although, the method is returning the non-generic JsonConverter base type and not the generic JsonConverter<T> so the comments in source might be misleading.
@steveharter - what do you think this should be doc'd as? Why are we talking about the T here for a non-generic type?
| <param name="index">To be added.</param> | ||
| <summary>Gets the value at a specified index when the current value is an <see cref="F:System.Text.Json.JsonValueKind.Array" />.</summary> | ||
| <value>To be added.</value> | ||
| <param name="index">The item index.</param> |
There was a problem hiding this comment.
| <param name="index">The item index.</param> | |
| <param name="index">The zero-based item index within the JSON array.</param> |
| </Parameters> | ||
| <Docs> | ||
| <param name="other">To be added.</param> | ||
| <param name="other">The object to compare to this instance.</param> |
There was a problem hiding this comment.
This API doesn't take object, but rather JsonEncodedText.
| <param name="other">The object to compare to this instance.</param> | |
| <param name="other">The JSON encoded text to compare to this instance.</param> |
3 participants
They were either not documented in source or had inheritdoc.