Skip to content

Conversation

@Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Nov 12, 2024

Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

The changes are OK but it's hard to find anything in them that is substantially different in meaning than the original (except the missing TOC node); the text generally seems fine without these changes.

Copy link
Member

@DeagleGross DeagleGross left a comment

Choose a reason for hiding this comment

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

We have also changed the name to Consider using ListenAnyIP() instead of Listen(IPAddress.Any) (see PR notes)

can you please change the title \ description of the analyzer as well?

## Cause

On the server machine that supports IPv6, listening to `Any`, rather than `IPv6Any` will either not work or be slower than necessary, because of the [underlying System types implementation](https://github.com/dotnet/runtime/issues/82404).
[IPv6Any](/dotnet/api/system.net.ipaddress.ipv6any) is preferred to [Any](/dotnet/api/system.net.ipaddress.any) because `Any` is less performant than `IPv6Any`. In some cases, `Any` may not work at all. `Any` has performance problems due to the [underlying System types implementation](https://github.com/dotnet/runtime/issues/82404).
Copy link
Member

Choose a reason for hiding this comment

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

phrasing has performance problems is not what I wanted to say - it raises a question of why not fix the System types then? I wanted to avoid this

Copy link
Contributor Author

@Rick-Anderson Rick-Anderson Nov 13, 2024

Choose a reason for hiding this comment

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

phrasing has performance problems is not what I wanted to say - it raises a question of why not fix the System types then? I wanted to avoid this

I've reverted to your wording with my wording. I don't see the difference between slower than necessary and less performant.

## Cause

On the server machine that supports IPv6, listening to `Any`, rather than `IPv6Any` will either not work or be slower than necessary, because of the [underlying System types implementation](https://github.com/dotnet/runtime/issues/82404).
[IPv6Any](/dotnet/api/system.net.ipaddress.ipv6any) is preferred to [Any](/dotnet/api/system.net.ipaddress.any) because `Any` is less performant than `IPv6Any`. In some cases, `Any` may not work at all. `Any` has performance problems due to the [underlying System types implementation](https://github.com/dotnet/runtime/issues/82404).
Copy link
Member

Choose a reason for hiding this comment

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

lets also include the info about server machine supporting IPv6 - it is the case where the analyzer should be applied. Its necessary to have this information

@guardrex
Copy link
Collaborator

@Rick-Anderson ... I opened a PR for it, but it's best if you fix it here ...

ms.author: deaglegross

... throws a build warning.

I'll close the PR that I opened.

@guardrex guardrex mentioned this pull request Nov 13, 2024
Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

Looks good. I left a few suggestions and one question.

Co-authored-by: Tom Dykstra <tdykstra@microsoft.com>
@Rick-Anderson Rick-Anderson marked this pull request as ready for review November 13, 2024 23:08
Rick-Anderson and others added 2 commits November 13, 2024 15:38
Co-authored-by: Tom Dykstra <tdykstra@microsoft.com>
author: deaglegross
monikerRange: '>= aspnetcore-10.0'
ms.author: deaglegross
ms.author: dmkorolev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rick-Anderson Rick-Anderson merged commit 0ddfbb8 into main Nov 18, 2024
3 checks passed
@Rick-Anderson Rick-Anderson deleted the asp026/ra/1 branch November 18, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up and Connect ASP0028 to TOC

5 participants