Test-Connection - Remove informational output, consolidate Ping usage #10478
Conversation
Rather than instantiate a new Ping() every time, we can store it in a readonly field and just call Send() as needed.
Intention was to write to Host. PowerShell implements this over information stream. |
|
That is true... I was thinking of a use case where Though I suppose the verbose output probably should just be something along the lines of: |
|
Yes, verbose output should has another form and probably be in other places because it serves other purposes. We could implement this later if this will be needed. |
|
I'll do a revision of the verbose output today, keeping it pretty simple. We can expand on it more at a later date if we need more. :) |
|
You could simply remove the extra output in the PR and add verbose in follow PR to speed up code review. |
|
@iSazonov would it be better to also remove progress bars in this PR or following PR? EDIT: It seems that change is a bit simpler at least from looking at the diffs, since we can just remove entirely the methods that handle the information/progress data. If you'd rather I leave the progress in for now, just ping me and I'll make the change. 🙂 |
:burn: Remove unused resource strings
src/Microsoft.PowerShell.Commands.Management/commands/management/TestConnectionCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Management/commands/management/TestConnectionCommand.cs
Outdated
Show resolved
Hide resolved
|
@TravisEz13 is there anything more you guys need from me on this one? I have a couple more PRs lined up and ready for this cmdlet that I'd love to get in for PSv7 to more fully cover the scope of the approved RFC. 💖 |
|
@SteveL-MSFT @TravisEz13 I know you guys are pretty busy at this point in the release cycle, but can't hurt to ask I hope: Can I ask to get this one merged in and move forward with implementing the remainder of the changes already approved in the RFC for v7 timeframe? I have the code for it all written (at least one or maybe two more PR's worth) and the RFC is already approved, so I would like to have all the Test-Connection changes I plan on making merged in for v7 GA if at all possible. Thank you! You guys are awesome! 😊 💖 |
|
Did the output of the Cmdlet change in this PR? |
|
@TravisEz13 The success output of this cmdlet has not changed. That is what I plan to change in a follow-up PR. 🙂 Or, if you'd prefer, I can merge the changes from that branch into here directly, but that does make the diffs unfortunately difficult to work through, which is why I subdivided my PRs on this one. 🙂 |
|
No... I prefer it like this. I'm just trying to clarify. |
|
🎉 Handy links: |
PR Summary
Replaces allRemoves all verbose and progress output. We will revisit verbose output in a later PR.WriteInformation()calls withWriteVerbose()Pingobject which is reused for allSend()calls.IDisposableforTest-Connectionso that it can dispose of thePingobject when it is no longer needed.PR Context
Part 2 of #10044, refactor of
Test-Connectioncmdlet.Fixes #6768
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.