Skip to content

Added documentation for APIs in System.Net namespace introduced in 2.x #3280

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 4 commits into from
Nov 23, 2019
Merged

Added documentation for APIs in System.Net namespace introduced in 2.x #3280

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2648b07
Added documentation for APIs in System.Net namespace introduced in 2.x
jozkee Oct 2, 2019
c0d7a49
Apply some suggestions from code review
jozkee Nov 21, 2019
592b8db
avoid CI edit
mairaw Nov 22, 2019
e274e0c
Address moving paragraph from remarks to param tag
jozkee Nov 23, 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
14 changes: 11 additions & 3 deletions xml/System.Net/FtpWebResponse.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,17 @@
<ReturnType>System.String</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<remarks>To be added.</remarks>
<summary>Always throws a <see cref="T:System.NotImplementedException" />.</summary>
<value>Always throws a <see cref="T:System.NotImplementedException" />.</value>
<remarks>
<format type="text/markdown"><![CDATA[

## Remarks
This property is provided only for compatibility with other implementations of the <xref:System.Net.WebRequest> and <xref:System.Net.WebResponse> classes. There is no reason to use it.

]]></format>
</remarks>
<exception cref="T:System.NotImplementedException">Content type information is not implemented for FTP.</exception>
</Docs>
</Member>
<Member MemberName="ExitMessage">
Expand Down
4 changes: 2 additions & 2 deletions xml/System.Net/HttpVersion.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
<ReturnType>System.Version</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<summary>Defines a <see cref="T:System.Version" /> instance for an unknown HTTP version.</summary>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -196,7 +196,7 @@
<ReturnType>System.Version</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<summary>Defines a <see cref="T:System.Version" /> instance for HTTP 2.0.</summary>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down
74 changes: 52 additions & 22 deletions xml/System.Net/IPAddress.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,22 @@
<Parameter Name="address" Type="System.ReadOnlySpan&lt;System.Byte&gt;" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="address">To be added.</param>
<summary>To be added.</summary>
<remarks>To be added.</remarks>
<param name="address">The byte span value of the IP address.</param>
<summary>Initializes a new instance of the <see cref="T:System.Net.IPAddress" /> class with the address specified as a byte span.</summary>
<remarks>
<format type="text/markdown"><![CDATA[

## Remarks
The <xref:System.Net.IPAddress> is created with the <xref:System.Net.IPAddress.Address> property set to `address`.

If the length of `address` is 4, this method constructs an IPv4 address; otherwise, an IPv6 address with a scope of 0 is constructed.

The byte span is assumed to be in network byte order with the most significant byte first in index position 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be documented in the param rather than remarks?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1
This is an interesting suggestion. We should always keep in mind that remarks do not show up in intellisense, so if we add this information at the end of the param, we ensure developers have this information available first hand.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's one reason, but also these remarks are all specific to the one param.


]]></format>
</remarks>
<exception cref="T:System.ArgumentException">
<paramref name="address" /> contains a bad IP address.</exception>
</Docs>
</Member>
<Member MemberName=".ctor">
Expand Down Expand Up @@ -285,10 +298,25 @@
<Parameter Name="scopeid" Type="System.Int64" Index="1" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="address">To be added.</param>
<param name="scopeid">To be added.</param>
<summary>To be added.</summary>
<remarks>To be added.</remarks>
<param name="address">The byte span value of the IP address.</param>
<param name="scopeid">The long value of the scope identifier.</param>
<summary>Initializes a new instance of the <see cref="T:System.Net.IPAddress" /> class with the address specified as a byte span and the specified scope identifier.</summary>
<remarks>
<format type="text/markdown"><![CDATA[

## Remarks
This constructor instantiates an IPv6 address. The `scopeid` identifies a network interface in the case of a link-local address. The scope is valid only for link-local and site-local addresses.

The byte span is assumed to be in network byte order with the most significant byte first in index position 0.

]]></format>
</remarks>
<exception cref="T:System.ArgumentException">
<paramref name="address" /> contains a bad IP address.</exception>
<exception cref="T:System.ArgumentOutOfRangeException">
<paramref name="scopeid" /> &lt; 0
- or -
<paramref name="scopeid" /> &gt; 0x00000000FFFFFFFF</exception>
</Docs>
</Member>
<Member MemberName="Address">
Expand Down Expand Up @@ -1686,10 +1714,12 @@
<Parameter Name="ipString" Type="System.ReadOnlySpan&lt;System.Char&gt;" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="ipString">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="ipString">A character span that contains an IP address in dotted-quad notation for IPv4 and in colon-hexadecimal notation for IPv6.</param>
<summary>Converts an IP address represented as a character span to an IPAddress instance.</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use the language "converts" elsewhere in docs when we are parsing a string?

Copy link
Member Author

Choose a reason for hiding this comment

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

@scalablecory It is used in the other method signature, also it is used in IPEndPoint.Parse and basically in any other type, int in example

<returns>An <see cref="T:System.Net.IPAddress" /> instance.</returns>
<remarks>To be added.</remarks>
<exception cref="T:System.FormatException">
<paramref name="ipString" /> is not a valid IP address.</exception>
</Docs>
</Member>
<Member MemberName="Parse">
Expand Down Expand Up @@ -1917,10 +1947,10 @@
<Parameter Name="charsWritten" Type="System.Int32" RefType="out" Index="1" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="destination">To be added.</param>
<param name="charsWritten">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="destination">When this method returns, the IP address as a span of characters.</param>
<param name="charsWritten">When this method returns, the number of characters written into the span.</param>
<summary>Tries to format the current IP address into the provided span.</summary>
<returns><see langword="true" /> if the formatting was successful; otherwise, <see langword="false" />.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -1954,10 +1984,10 @@
<Parameter Name="address" Type="System.Net.IPAddress" RefType="out" Index="1" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="ipString">To be added.</param>
<param name="address">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="ipString">The byte span to validate.</param>
<param name="address">The <see cref="T:System.Net.IPAddress" /> version of the byte span.</param>
<summary>Determines whether a byte span represents a valid IP address.</summary>
<returns><see langword="true" /> if <paramref name="ipString" /> was able to be parsed as an IP address; otherwise, <see langword="false" />.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -2044,10 +2074,10 @@
<Parameter Name="bytesWritten" Type="System.Int32" RefType="out" Index="1" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="destination">To be added.</param>
<param name="bytesWritten">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="destination">When this method returns, the IP address as a span of bytes.</param>
Copy link
Contributor

@scalablecory scalablecory Oct 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
<param name="destination">When this method returns, the IP address as a span of bytes.</param>
<param name="destination">When this method returns <see langword="true" />, receives the byte representation of the IP address in network byte order.</param>

Copy link
Member Author

@jozkee jozkee Oct 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I agree but shouldn't it be "contains" instead of "receives"? I also agree on the "When this method returns <see langword="true" />" part but that is not the pattern that other TryParse APIs follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

"contains" to me is a little confusing because it sounds like a precondition to calling this method. "receives" tells me it will have the IP address written to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mairaw thoughts on true vs langword true?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to say contains. So what happens when the method returns false?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mairaw if TryWriteBytes return false, it means that it didn't write into destination.

Copy link
Contributor

Choose a reason for hiding this comment

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

thoughts on true vs langword true?

@scalablecory Your suggestion is correct: we should use <see langword="true" /> instead of just true.

+1 What @mairaw said: We should mention what the contents of destination will be if the method returns false.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be okay leaving without true or false as well. I just wouldn't say
"When this method returns true, ..."

I'd simply say "When this method returns, "

People should be checking the result of the method anyway before trying to use these, I'd say

<param name="bytesWritten">When this method returns, the number of bytes written into the span.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<param name="bytesWritten">When this method returns, the number of bytes written into the span.</param>
<param name="bytesWritten">When this method returns <see langword="true" />, receives the number of bytes written into the span.</param>

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this suggestion is correct. bytesWritten will always have a number, whether the method returns true or false. Usually these methods set this argument to 0 right before they return false. If that's the case, there's no point in mentioning "When this method returns true|false".

What @jozkee wrote might be enough, if he can confirm that the method sets this to zero on failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the original version better. So if it's simply 0 when false, the original sentence still looks good.

<summary>Tries to write the current IP address into a span of bytes.</summary>
<returns><see langword="true" /> if the IP address is sucessfully written to the given span; otherwise, <see langword="false" />.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down