Skip to content

Document various isolated System.Text.Json elements #2910

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

Merged
merged 3 commits into from
Aug 2, 2019
Merged

Document various isolated System.Text.Json elements #2910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9a67ed
Document various isolated System.Text.Json elements
carlossanlop Aug 1, 2019
cd1ffcc
Update JsonStringEnumConverter.xml
mairaw Aug 2, 2019
0e4b02e
Update JsonElement.xml
mairaw Aug 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions xml/System.Text.Json.Serialization/JsonStringEnumConverter.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ Reading is case insensitive. Writing can be customized by using a <xref:System.T
<Parameter Name="typeToConvert" Type="System.Type" />
</Parameters>
<Docs>
<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>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

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:

Suggested change
<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>

cc @JeremyKuhne, @steveharter

<summary>Determines whether the specified type can be converted to an enum.</summary>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

enum -> enumeration

Suggested change
<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>

<returns><see langword="true" /> if the type can be converted; otherwise, <see langword="false" />.</returns>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

Similarly here (from #2889 (comment)):
if the type can be converted; otherwise, .

cc @jozkee

<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -109,9 +109,9 @@ Reading is case insensitive. Writing can be customized by using a <xref:System.T
<Parameter Name="typeToConvert" Type="System.Type" />
</Parameters>
<Docs>
<param name="typeToConvert">To be added.</param>
<summary>To be added.</summary>
<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>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

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?

<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down
6 changes: 3 additions & 3 deletions xml/System.Text.Json/JsonElement.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,9 +837,9 @@ This method does not parse the contents of a JSON string value.
<Parameter Name="index" Type="System.Int32" />
</Parameters>
<Docs>
<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>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

Suggested change
<param name="index">The item index.</param>
<param name="index">The zero-based item index within the JSON array.</param>

<summary>Gets the value at the specified index if the current value is an <see cref="F:System.Text.Json.JsonValueKind.Array" />.</summary>
<value>The value at the specified index.</value>
<remarks>To be added.</remarks>
<exception cref="T:System.InvalidOperationException">This value's <see cref="P:System.Text.Json.JsonElement.ValueKind" /> is not <see cref="F:System.Text.Json.JsonValueKind.Array" />.</exception>
<exception cref="T:System.IndexOutOfRangeException"><paramref name="index" /> is not in the range [0, <see cref="M:System.Text.Json.JsonElement.GetArrayLength" />()).</exception>
Expand Down
6 changes: 3 additions & 3 deletions xml/System.Text.Json/JsonEncodedText.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ This type can be used to cache and store known strings used for writing JSON ahe
<Docs>
<param name="value">The value to be transformed as JSON encoded text.</param>
<summary>Encodes the string text value as a JSON string.</summary>
<returns>To be added.</returns>
<returns>The string encoded as a JSON string.</returns>
<remarks>To be added.</remarks>
<exception cref="T:System.ArgumentNullException"><paramref name="value" /> is <see langword="null" />.</exception>
<exception cref="T:System.ArgumentException"><paramref name="value" /> is too large or contains invalid UTF-16 characters.</exception>
Expand Down Expand Up @@ -154,7 +154,7 @@ This type can be used to cache and store known strings used for writing JSON ahe
<Parameter Name="obj" Type="System.Object" />
</Parameters>
<Docs>
<param name="obj">To be added.</param>
<param name="obj">The object to compare to this instance.</param>
<summary>Determines whether this instance and a specified object, which must also be a <see cref="T:System.Text.Json.JsonEncodedText" /> instance, have the same value.</summary>
<returns><see langword="true" /> if the current instance and <paramref name="obj" /> are equal; otherwise, <see langword="false" />.</returns>
<remarks>
Expand Down Expand Up @@ -190,7 +190,7 @@ If `obj` is `null`, the method returns `false`.
<Parameter Name="other" Type="System.Text.Json.JsonEncodedText" />
</Parameters>
<Docs>
<param name="other">To be added.</param>
<param name="other">The object to compare to this instance.</param>
Copy link
Contributor

@ahsonkhan ahsonkhan Aug 2, 2019

Choose a reason for hiding this comment

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

This API doesn't take object, but rather JsonEncodedText.

Suggested change
<param name="other">The object to compare to this instance.</param>
<param name="other">The JSON encoded text to compare to this instance.</param>

<summary>Determines whether this instance and another specified <see cref="T:System.Text.Json.JsonEncodedText" /> instance have the same value.</summary>
<returns><see langword="true" /> if this instance and <paramref name="other" /> have the same value; otherwise, <see langword="false" />.</returns>
<remarks>
Expand Down