Using ParameterSet prefixes looks good to me - this makes code more readable and understandable.
Otherwise a code reader will have to review all code in the file to find that a const is used in Parameter() attribute as a parameter set.

I'd remove the commit.

Is the <Name>Set form not sufficient to convey that intention here? Every context they're used is either in a [Parameter] attribute or in a switch (ParameterSetName) construct -- both of which are more than sufficient to indicate that they're used in parameter sets.

Since attribute values must be constant values, I think it's reasonable to assume that readers will be able to infer this information. (Plus, most code editors have a "Show/Peek Definition" in their right-click context menu, and these constants are defined immediately below here, at the top of the class.)

As I was reading through this file I found that the overly long names of the constants tended to make them confusing to read, and they're typically surrounded by at least one mention of [Parameter] or something similar, rendering the Parameter segment of the name pretty redundant in context.

It's also not a pattern I see anywhere else in the repo?

C# best practice is to use informative names without abbreviations. As result we sometimes have long names.
I see different patterns in our code. You could look AddMember.cs, AddType.cs, ConvertFromMarkDownCommand.cs.
It would be nice to have one patter but we haven't.
As for my request one example is that we frequently use switch statements for parameter sets and it would be good at first glance to understand that a constant with the parameter set name is used there.

I guess that makes some sense, and yeah I hear you on the inconsistent patterns there. With the switch statements, these are usually used as switch (ParameterSetName) {}, so I think it's pretty clear there as well given that that's the context of the switch statement.

Adjusted the names here to be clearer. 🙂 👍

I'd do not remove this - later we could add new parameter sets in the cmdlet. I did not implement them because of bugs in CoreFX.

Which possibilities exist that would be agnostic to IPv4/v6?

I don't understand you question. I mean that later it would be easier to add new features in the cmdlet (like PathPing). (I am thinking about enhanced version of the cmslet with remoting support: user in A point could ping/traceroute/... C from B)

Is there significant advantage to having that from this cmdlet? This would have to be supported by PSRemoting anyway, so I don't know why we'd do that here -- users have Invoke-Command already, and the native APIs utilised by .NET core (and by extension, us) don't have this capability, so we'd be leaning on that already.

Basically, I'm asking -- why do these parameters need to have the sets specified when not all parameters are written this way? I think it makes more sense to have the command written to its current capability, and then when we implement new features we can add them or adjust parameters as needed.

Can we reasonably predict possible future scenarios here with accuracy? I don't think it makes a lot of sense to complicate parameter binding here at this point in time. 😕

🤷‍♂ not particularly an issue I suppose, we could debate this all day. It isn't hurting anything by leaving it in; I reverted this commit.

Thanks! 🙂

Thanks!

This would have to be supported by PSRemoting anyway

It seems we have a tracking issue to have remoting common parameters for all cmdlets.
With this we could get functionality like Test-NetConnection.

That would be awesome!

Something to note: The original implementation has a -Source command. It seems the win32 APIs for ICMP echo actually support a similar parameter, but there it is used to designate the NIC that is used to send the ICMP request.

However, .NET Core does not expose this capability to us at present. Hopefully we can get that in future as well! 😊

It seems the win32 APIs for ICMP echo actually support a similar parameter,

It is WMI.

It do not make the code more readable for me.
I'd remove the commit.

Why do we need the new line? I'd revert.

Mostly readability, long lines tend to get lost in the mix here. It's also a common pattern with a fair few cmdlets in the repo to put the HelpUri on the following line.

🤷‍♂️

I believe text width up to 120-144 is good for modern displays.

Sounds good, I'll revert this one. 🙂

Ilya (@iSazonov) the width here if this stays on a single line is 150; should I leave this on two lines? 😕

My request is not blocking. If you see a problem on your screen - make two line or multi-line format.

I think I'll leave this one as-is for now. Two lines is (I think) sufficient. Thanks! 🙂