Add confirmation prompts and examples for Remove- functions by giuseppecampanelli · Pull Request #174 · microsoft/PowerShellForGitHub

Giuseppe Campanelli (giuseppecampanelli) · 2020-05-19T22:52:43Z

Fixes #171

Microsoft Contribution License Agreements (msftclas) · 2020-05-19T22:52:56Z

All CLA requirements met.

Howard Wolosky (HowardWolosky)

Thanks so much for this, @themilanfan!

Really minor feedback to address, and then I'll be happy to merge this in. Thanks again, and welcome to the project!

Howard Wolosky (HowardWolosky)

Actually, just realized that you didn't add the necessary ShouldProcess commands to have any impact here.

See

PowerShellForGitHub/GitHubProjects.ps1

Line 496 in d279f1c

if ($PSCmdlet.ShouldProcess($project, "Remove project"))

You probably need to update a bunch of tests too in order for them to pass (adding -Confirm:$false to those calls), because once this is done, all of the existing tests that make use of Remove-* commands will fail because they'll hang waiting for user confirmation.

…pt for confirmation Also re-addded needed whitespaces

Added line-space in example

Giuseppe Campanelli (giuseppecampanelli) · 2020-05-20T00:22:44Z

Howard Wolosky (@HowardWolosky) thank you for the feedback, I've just committed the changes!

Giuseppe Campanelli (giuseppecampanelli)

Completed in new commits

Giuseppe Campanelli (giuseppecampanelli)

Completed review in commits

Giuseppe Campanelli (giuseppecampanelli) · 2020-05-21T01:29:51Z

Howard Wolosky (@HowardWolosky) I'm not sure what's happening with the check, I reviewed all the changes you requested and thought it would resume

Howard Wolosky (HowardWolosky) · 2020-05-22T13:57:23Z

@themilanfan - Thanks for the update. Will review the changes this weekend.

As for the checks, ignore them for now. I've been running into issues with the tests properly running in PR CI builds, so I still need to verify them locally before merging. I'll get to fixing the PR CI build at some point.

Howard Wolosky (HowardWolosky)

Overall, this looks like a great update, thanks!
One minor documentation suggestion in this review.
Additionally, more tests have been added since you authored this PR. Would you be willing to refresh this PR with the latest changes from master, and incorporate any remaining -Confirm:$false parameters into any new instances of Remove-* methods being called in the tests?

Thanks so much!

…ons-remove-funcs

Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>

Giuseppe Campanelli (giuseppecampanelli) · 2020-05-26T22:37:29Z

Hi Howard Wolosky (@HowardWolosky), I've added the remaining -Confirm:$false as well as updated the documentation and fixed another small issue in GitHubLabels.tests.ps1 as well. I will also wait for this to merge so that for issue #176 I don't miss any Confirm:$false there.

Giuseppe Campanelli (giuseppecampanelli)

Changes reviewed

Giuseppe Campanelli (giuseppecampanelli) · 2020-05-29T01:01:27Z

Removed them 😄

Howard Wolosky (HowardWolosky) · 2020-05-30T23:25:09Z

It looks like some conflicts came in as a result of some of the recent commits as I've been trying to get the CI to run clean on all platforms. Can you please address the conflicts? Then I'll give it a run through the CI and get it committed....Thanks!

…ons-remove-funcs

Giuseppe Campanelli (giuseppecampanelli) · 2020-06-01T13:15:45Z

Resolved conflicts 😄

Howard Wolosky (HowardWolosky) · 2020-06-01T16:08:21Z

/azp run PowerShellForGitHub-CI

azure-pipelines · 2020-06-01T16:08:31Z

Azure Pipelines successfully started running 1 pipeline(s).

Howard Wolosky (HowardWolosky)

This is ready to go. Thanks so much!

Add confirmation prompts and examples for Remove- functions

86ab2ac

Howard Wolosky (HowardWolosky) suggested changes May 19, 2020

View reviewed changes

Comment thread GitHubLabels.ps1 Outdated

Comment thread GitHubAssignees.ps1

Howard Wolosky (HowardWolosky) suggested changes May 19, 2020

View reviewed changes

Giuseppe Campanelli (giuseppecampanelli) added 2 commits May 19, 2020 20:21

Added missing ShouldProcess checks and updated test cases to not prom…

143060d

…pt for confirmation Also re-addded needed whitespaces

Missed file

67e7b0e

Added line-space in example

Giuseppe Campanelli (giuseppecampanelli) commented May 21, 2020

View reviewed changes

Howard Wolosky (HowardWolosky) self-assigned this May 22, 2020

Howard Wolosky (HowardWolosky) added the technical debt Work that was postponed by a previous change. label May 22, 2020

Howard Wolosky (HowardWolosky) suggested changes May 26, 2020

View reviewed changes

Comment thread GitHubRepositories.ps1 Outdated

Giuseppe Campanelli (giuseppecampanelli) and others added 4 commits May 26, 2020 18:25

Merge remote-tracking branch 'upstream/master' into prompt-confirmati…

454784f

…ons-remove-funcs

Added remaining confirmations for Remove-GitHubRepository

ad28bf9

Update GitHubRepositories.ps1

5d0e3dc

Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>

Fix issue in GitHubLabels.tests.ps1

88fedc7

Giuseppe Campanelli (giuseppecampanelli) mentioned this pull request May 27, 2020

Add tests to GitHubRepositories.tests.ps1 #176

Merged

Giuseppe Campanelli (giuseppecampanelli) commented May 27, 2020

View reviewed changes

Howard Wolosky (HowardWolosky) reviewed May 28, 2020

View reviewed changes

Comment thread GitHubAssignees.ps1 Outdated

Remove exception for PSShouldProcess

86d2086

Merge remote-tracking branch 'upstream/master' into prompt-confirmati…

599d855

…ons-remove-funcs

Merge branch 'master' into prompt-confirmations-remove-funcs

828a828

Howard Wolosky (HowardWolosky) approved these changes Jun 1, 2020

View reviewed changes

Howard Wolosky (HowardWolosky) merged commit a6a27aa into microsoft:master Jun 1, 2020

Howard Wolosky (HowardWolosky) mentioned this pull request Jun 2, 2020

Add GitHub Reactions support #193

Merged

Howard Wolosky (HowardWolosky) added the breaking change This includes a change in existing published module behavior. label Jun 2, 2020

Simon Heather (X-Guardian) mentioned this pull request Jun 7, 2020

PowerShellForGitHub: Suggested WhatIf/Confirm Processing Improvements #225

Closed

Uh oh!

Conversation

Giuseppe Campanelli (giuseppecampanelli) commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Howard Wolosky (HowardWolosky) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Howard Wolosky (HowardWolosky) left a comment

Choose a reason for hiding this comment

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) commented May 20, 2020

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) left a comment

Choose a reason for hiding this comment

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) left a comment

Choose a reason for hiding this comment

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Howard Wolosky (HowardWolosky) commented May 22, 2020

Uh oh!

Howard Wolosky (HowardWolosky) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) commented May 26, 2020

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) commented May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Howard Wolosky (HowardWolosky) commented May 30, 2020

Uh oh!

Giuseppe Campanelli (giuseppecampanelli) commented Jun 1, 2020

Uh oh!

Howard Wolosky (HowardWolosky) commented Jun 1, 2020

Uh oh!

azure-pipelines Bot commented Jun 1, 2020

Uh oh!

Howard Wolosky (HowardWolosky) left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Giuseppe Campanelli (giuseppecampanelli) commented May 19, 2020 •

edited

Loading

Microsoft Contribution License Agreements (msftclas) commented May 19, 2020 •

edited

Loading

Giuseppe Campanelli (giuseppecampanelli) commented May 21, 2020 •

edited

Loading

Giuseppe Campanelli (giuseppecampanelli) left a comment •

edited

Loading

Giuseppe Campanelli (giuseppecampanelli) commented May 29, 2020 •

edited

Loading