FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Refactor and implement `Restart-Computer` for `Un*x` and macOS by brendandburns · Pull Request #11319 · PowerShell/PowerShell · GitHub

/ PowerShell Public

Refactor and implement Restart-Computer for Un*x and macOS - #11319

Merged
Travis Plunk (TravisEz13) merged 6 commits into
PowerShell:masterfrom
brendandburns:restarter
Feb 3, 2020
ConversationCommits (6)ChecksFiles changed
Merged

Refactor and implement Restart-Computer for Un*x and macOS#11319
Travis Plunk (TravisEz13) merged 6 commits into
PowerShell:masterfrom
brendandburns:restarter

Conversation

Brendan Burns (brendandburns) commented Dec 11, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Implement the Restart-Computer command for Un*x and MacOS

PR Context

Compliment to the recent implementation of Stop-Computer (#11151)

PR Checklist

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Dec 11, 2019
Ilya (iSazonov) added this to the 7.1.0-preview.1 milestone Dec 11, 2019

Ilya (iSazonov) commented Dec 11, 2019

Copy link
Copy Markdown
Collaborator

Brendan Burns (@brendandburns) Please don't rebase until maintainers ask you. Rebasing complicates reviewing.

Steve Lee (SteveL-MSFT) requested changes Dec 11, 2019
View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The Windows Computer cmdlets already use a test hook for testing, should use the same thing here. See https://github.com/PowerShell/PowerShell/blob/master/src/Microsoft.PowerShell.Commands.Management/commands/management/Computer.cs#L2130

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 11, 2019

Ilya (iSazonov) commented Dec 11, 2019

Copy link
Copy Markdown
Collaborator

We merged #11151 without new tests.

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 11, 2019

Brendan Burns (brendandburns) commented Dec 11, 2019

Copy link
Copy Markdown
Contributor Author

I will address comments and add tests for both commands.

Thanks

Brendan Burns (brendandburns) commented Dec 11, 2019

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) is there a preferred style of addressing comments for the PowerShell repo? Should I just add additional commits instead of squashing commits?

(I'm new here :)

Thanks

Ilya (iSazonov) commented Dec 11, 2019

Copy link
Copy Markdown
Collaborator

Should I just add additional commits instead of squashing commits?

Yes, please add new commits. Some commits or one for all comments.

Steve Lee (SteveL-MSFT) commented Dec 11, 2019

Copy link
Copy Markdown
Member

Brendan Burns (@brendandburns) unless there is a need to preserve history, we always squash on merge. For the sake of the reviewers, it's always been easier to add new commits so we can see what exactly changed.

Ilya (iSazonov) reviewed Dec 13, 2019
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Stop-Computer.Tests.ps1 Outdated

Ilya (iSazonov) Dec 13, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please remove the commented code.

Brendan Burns (brendandburns) Jan 15, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done.

Brendan Burns (brendandburns) commented Jan 15, 2020

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Steve Lee (@SteveL-MSFT)

Sorry it took so long for me to get back to this. I've added tests for both Restart-Computer and Stop-Computer on Unix.

Please take another look.

Thanks!

Ilya (iSazonov) reviewed Jan 15, 2020
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Restart-Computer.Tests.ps1 Outdated
Comment on lines 31 to 34

Ilya (iSazonov) Jan 15, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please use It "Should support -computer parameter" -Skip:(!$IsWindows) {

Brendan Burns (brendandburns) Jan 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done for all.

Ilya (iSazonov) reviewed Jan 15, 2020
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Restart-Computer.Tests.ps1 Outdated
Comment on lines 40 to 43

Ilya (iSazonov) Jan 15, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The same. Below too.

Brendan Burns (brendandburns) Jan 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done for all.

Brendan Burns (brendandburns) commented Jan 16, 2020

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) comments addressed, please re-check.

Thanks!

Ilya (iSazonov) commented Jan 16, 2020

Copy link
Copy Markdown
Collaborator

Brendan Burns (@brendandburns) Did you address all Steve Lee (@SteveL-MSFT) 's comments?

Brendan Burns (brendandburns) commented Jan 16, 2020

Copy link
Copy Markdown
Contributor Author

yes, I verified in the code that I had addressed both of the comments from Steve Lee (@SteveL-MSFT)

Thanks

Ilya (iSazonov) commented Jan 17, 2020

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) Please update your review.

1 similar comment

Travis Plunk (TravisEz13) commented Jan 28, 2020

Copy link
Copy Markdown
Member

Steve Lee (@SteveL-MSFT) Please update your review.

Travis Plunk (TravisEz13) commented Feb 3, 2020

Copy link
Copy Markdown
Member

Steve Lee (@SteveL-MSFT) Please update your review

Travis Plunk (TravisEz13) commented Feb 3, 2020

Copy link
Copy Markdown
Member

The merge is bad. I'll fix it.

Travis Plunk (TravisEz13) commented Feb 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Rebased to resolve conflicts and changed #region format to match changes from conflict

Travis Plunk (TravisEz13) commented Feb 3, 2020

Copy link
Copy Markdown
Member

PoshChan-Bot (@PoshChan) Please remind me in 1 hour

PoshChan-Bot (PoshChan) commented Feb 3, 2020

Copy link
Copy Markdown
Collaborator

Travis Plunk (@TravisEz13), this is the reminder you requested 1 hour ago

Travis Plunk (TravisEz13) changed the title Refactor and implement Restart-Computer for Un*x and MacOS Refactor and implement Restart-Computer for Un*x and macOS Feb 3, 2020
Travis Plunk (TravisEz13) merged commit 127fec5 into PowerShell:master Feb 3, 2020

Ilya (iSazonov) commented Feb 4, 2020

Copy link
Copy Markdown
Collaborator

Brendan Burns (@brendandburns) Thanks for your contribution!

Deleted user (ghost) commented Mar 26, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sean Wheeler (sdwheeler) mentioned this pull request May 20, 2020
Closed
12 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Refactor and implement Restart-Computer for Un*x and macOS (Power…
eb1be5b 
…Shell#11319)


Co-authored-by: Ilya <darpa@yandex.ru>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Ilya (iSazonov) iSazonov left review comments
Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw

Assignees

Travis Plunk (TravisEz13)

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.1

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL