Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables by KirkMunro · Pull Request #10317 · PowerShell/PowerShell · GitHub

PR Summary

As part of the cleanup of workflow code, this PR does the following:

Treats ActionPreference.Suspend enumeration value as not supported and reserved for future use.
Update internal logic dependent on ActionPreference.Suspend such that using that value results in appropriate error messages.
Remove unused resource strings.
Updated and added tests related to ActionPreference.Suspend to ensure that it is generating the errors we expect when it is used.
Remove unnecessary restriction around ActionPreference.Ignore (see this discussion in issue 4348). This change is made as part of this PR because it is in the exact same body of code, and it makes sense to include it here.

PR Context

See issue #9570.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
Breaking changes
- None
- OR
- Experimental feature(s) needed
  - Experimental feature name(s):
User-facing changes
- Not Applicable
- OR
- Documentation needed
  - Issue filed: MicrosoftDocs/PowerShell-Docs #4624
Testing - New and feature
Tooling
- I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
- OR
- I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
  - Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
  - Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
  - Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
  - Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

LGTM with one comment.

LGTM

@PowerShell/powershell-committee the decision here is what to do about removing a workflow specific value from the existing public enum. One option is to mark it obsolete.

@PowerShell/powershell-committee reviewed this, we discussed removal, using [obsolete] attribute, and leaving as-is. We discussed the possibility of using this value in the future for job control if there is a way to suspend a PSJob. For that reason, we agreed to leave this enum value. We also recommend that we explicitly number enum values so in the future, if we do decide to remove a value and add another one, then existing code will break/behavior appropriately.

To address the committee review feedback, I just made the following changes:

Put the enumerated value back.
Updated internal logic to generate appropriate errors about using an enumerated value that is not supported and reserved for future use.
Updated tests to ensure that it's behaving properly everywhere we need it to.

I'll also update the OP accordingly.

Note that regarding explicit numbering of the ActionPreference enumerated values, I have that handled in PR #8205.

Also, once either this PR or #8205 are merged in, I will update the other PR accordingly because there are some overlapping changes that I doubt will merge without manual intervention.

Just FYI in case it wasn't clear: I updated this PR once #8205 was merged into PowerShell, so this PR is one again ready for review/merge.

Two of the three Codacy issues are incorrect (looks like it needs to be updated for modern syntax), and the third one is related to a Pester test and will be ignored.

Paul Higinbotham (@PaulHigin) Could you please update your review (we have new commits)?

FYI, I will not be making changes based on the Codacy review because one of the issues is a false positive, and the other two are incorrect. This PR is ready for review by other reviewers.

Steve Lee (@SteveL-MSFT) PowerShell Committee request was addressed. Can we merge?

…orted, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables (PowerShell#10317)

🎉v7.0.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

…orted, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables (PowerShell#10317)

Kirk Munro (KirkMunro) added 2 commits August 7, 2019 20:10

remove deprecated ActionPreference.Suspend value

637ecbf

remove unused string

cd4b42c

Kirk Munro (KirkMunro) mentioned this pull request Aug 7, 2019

Remove references to ActionPreference.Suspend MicrosoftDocs/PowerShell-Docs#4624

Closed

8 tasks

Ilya (iSazonov) approved these changes Aug 8, 2019

View reviewed changes

Comment thread src/System.Management.Automation/engine/CommandBase.cs Outdated

Ilya (iSazonov) requested review from Paul Higinbotham (PaulHigin), Steve Lee (SteveL-MSFT), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) August 8, 2019 12:52

Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Aug 8, 2019

Ilya (iSazonov) added this to the 7.0.0-preview.3 milestone Aug 8, 2019

Ilya (iSazonov) self-assigned this Aug 8, 2019

Ilya (iSazonov) added the Breaking-Change breaking change that may affect users label Aug 8, 2019

Paul Higinbotham (PaulHigin) approved these changes Aug 8, 2019

View reviewed changes

Kirk Munro (KirkMunro) mentioned this pull request Aug 8, 2019

Code cleanup: bulk removal of deprecated workflow code #10223

Closed

15 tasks

Steve Lee (SteveL-MSFT) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Aug 19, 2019

Travis Plunk (TravisEz13) modified the milestones: 7.0.0-preview.3, 7.0.0-preview.4 Aug 19, 2019

Steve Lee (SteveL-MSFT) added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Aug 21, 2019

Ilya (iSazonov) mentioned this pull request Aug 22, 2019

Explicitly number enum values #10412

Closed

Kirk Munro (KirkMunro) added 2 commits August 22, 2019 14:58

Merge branch 'master' into deprecate-actionpreference-suspend

c53b9eb

put enum value back, updated plumbing and tests

1d84d26

CodeFactor changes

008d97b

Kirk Munro (KirkMunro) changed the title Remove deprecated ActionPreference.Suspend enumeration value Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state. Aug 22, 2019

PoshChan-Bot (PoshChan) reviewed Aug 22, 2019

View reviewed changes

Comment thread src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs

PoshChan-Bot (PoshChan) reviewed Aug 22, 2019

View reviewed changes

Comment thread src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs

remove global qualifier added during testing

d68b07b

Paul Higinbotham (PaulHigin) suggested changes Aug 29, 2019

View reviewed changes

Comment thread src/System.Management.Automation/engine/ExecutionContext.cs

Comment thread src/System.Management.Automation/engine/MshCommandRuntime.cs Outdated

Comment thread src/System.Management.Automation/engine/MshCommandRuntime.cs Outdated

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Aug 29, 2019

code review changes

5eb9658

Paul Higinbotham (PaulHigin) approved these changes Aug 29, 2019

View reviewed changes

Dongbo Wang (daxian-dbw) reviewed Sep 5, 2019

View reviewed changes

Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated

Comment thread src/System.Management.Automation/engine/ExecutionContext.cs

Dongbo Wang (daxian-dbw) added the Breaking-Change breaking change that may affect users label Sep 6, 2019

Kirk Munro (KirkMunro) added 2 commits September 6, 2019 00:33

Merge branch 'master' into deprecate-actionpreference-suspend

1962ea8

remove ActionPreference.Ignore restriction

7b0c053

Kirk Munro (KirkMunro) added a commit to KirkMunro/PowerShell that referenced this pull request Sep 6, 2019

move ActionPreference.Ignore fix to PowerShell#10317

9fda03f

Kirk Munro (KirkMunro) mentioned this pull request Sep 6, 2019

Add VerboseAction, DebugAction, ProgressAction, VerboseVariable, DebugVariable, and ProgressVariable common parameters #10238

Closed

14 tasks

Ilya (iSazonov) added CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log and removed CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log labels Sep 6, 2019

Dongbo Wang (daxian-dbw) approved these changes Sep 6, 2019

View reviewed changes

Dongbo Wang (daxian-dbw) removed Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log labels Sep 6, 2019

Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Sep 7, 2019

Ilya (iSazonov) merged commit 139cd94 into PowerShell:master Sep 7, 2019

Kirk Munro (KirkMunro) deleted the deprecate-actionpreference-suspend branch September 9, 2019 17:31

Dongbo Wang (daxian-dbw) mentioned this pull request Sep 25, 2019

Preventing setting 'Ignore' as the $ErrorActionPreference value is poorly enforced on the command line, and not at all in scripts #4348

Closed

Patrick Meinecke (SeeminglyScience) mentioned this pull request Dec 2, 2019

Isolate PSES dependencies from PowerShell on load + make PSES a pure binary module PowerShell/PowerShellEditorServices#1118

Merged

Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables by KirkMunro · Pull Request #10317 · PowerShell/PowerShell · GitHub

Uh oh!

Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables - #10317

Conversation

Kirk Munro (KirkMunro) commented Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Paul Higinbotham (PaulHigin) left a comment

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) commented Aug 21, 2019

Uh oh!

Steve Lee (SteveL-MSFT) commented Aug 21, 2019

Uh oh!

Kirk Munro (KirkMunro) commented Aug 22, 2019

Uh oh!

Kirk Munro (KirkMunro) commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kirk Munro (KirkMunro) commented Aug 26, 2019

Uh oh!

Ilya (iSazonov) commented Aug 27, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kirk Munro (KirkMunro) commented Sep 4, 2019

Uh oh!

Ilya (iSazonov) commented Sep 5, 2019

Uh oh!

Uh oh!

Uh oh!

Deleted user (ghost) commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Kirk Munro (KirkMunro) commented Aug 7, 2019 •
edited
Loading

Kirk Munro (KirkMunro) commented Aug 22, 2019 •
edited
Loading