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

KirkMunro · Aug 7, 2019

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

iSazonov

LGTM with one comment.

src/System.Management.Automation/engine/CommandBase.cs

PaulHigin

LGTM

SteveL-MSFT · Aug 21, 2019

@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.

SteveL-MSFT · Aug 21, 2019

@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.

KirkMunro · Aug 22, 2019

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.

KirkMunro · Aug 22, 2019

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.

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

KirkMunro · Aug 26, 2019

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.

iSazonov · Aug 27, 2019

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

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

src/System.Management.Automation/engine/MshCommandRuntime.cs

KirkMunro · Sep 4, 2019

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.

iSazonov · Sep 5, 2019

@SteveL-MSFT PowerShell Committee request was addressed. Can we merge?

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

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

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

ghost · Sep 19, 2019

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

Handy links:

Release Notes

KirkMunro added 2 commits August 7, 2019 20:10

remove deprecated ActionPreference.Suspend value

637ecbf

remove unused string

cd4b42c

KirkMunro mentioned this pull request Aug 7, 2019

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

Closed

8 tasks

iSazonov approved these changes Aug 8, 2019

View reviewed changes

src/System.Management.Automation/engine/CommandBase.cs Outdated Show resolved Hide resolved

iSazonov requested review from PaulHigin, SteveL-MSFT, adityapatwardhan and daxian-dbw August 8, 2019 12:52

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

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

iSazonov self-assigned this Aug 8, 2019

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

PaulHigin approved these changes Aug 8, 2019

View reviewed changes

KirkMunro mentioned this pull request Aug 8, 2019

Code cleanup: bulk removal of deprecated workflow code #10223

Closed

15 tasks

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

TravisEz13 modified the milestones: 7.0.0-preview.3, 7.0.0-preview.4 Aug 19, 2019

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

iSazonov mentioned this pull request Aug 22, 2019

Explicitly number enum values #10412

Closed

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

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 reviewed Aug 22, 2019

View reviewed changes

src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs Show resolved Hide resolved

PoshChan reviewed Aug 22, 2019

View reviewed changes

src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs Show resolved Hide resolved

iSazonov removed the Breaking-Change breaking change that may affect users label Aug 23, 2019

remove global qualifier added during testing

d68b07b

PaulHigin suggested changes Aug 29, 2019

View reviewed changes

src/System.Management.Automation/engine/ExecutionContext.cs Show resolved Hide resolved

src/System.Management.Automation/engine/MshCommandRuntime.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/MshCommandRuntime.cs Outdated Show resolved Hide resolved

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

PaulHigin approved these changes Aug 29, 2019

View reviewed changes

daxian-dbw reviewed Sep 5, 2019

View reviewed changes

test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/ExecutionContext.cs Show resolved Hide resolved

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

KirkMunro added 2 commits September 6, 2019 00:33

Merge branch 'master' into deprecate-actionpreference-suspend

1962ea8

remove ActionPreference.Ignore restriction

7b0c053

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

move ActionPreference.Ignore fix to PowerShell#10317

9fda03f

KirkMunro mentioned this pull request Sep 6, 2019

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

Closed

14 tasks

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

daxian-dbw approved these changes Sep 6, 2019

View reviewed changes

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

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

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

KirkMunro deleted the deprecate-actionpreference-suspend branch September 9, 2019 17:31

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

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

Search code, repositories, users, issues, pull requests...

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

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

Uh oh!

Conversation

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!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PaulHigin left a comment

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented Aug 21, 2019

Uh oh!

SteveL-MSFT commented Aug 21, 2019

Uh oh!

KirkMunro commented Aug 22, 2019

Uh oh!

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!

KirkMunro commented Aug 26, 2019

Uh oh!

iSazonov commented Aug 27, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KirkMunro commented Sep 4, 2019

Uh oh!

iSazonov commented Sep 5, 2019

Uh oh!

Uh oh!

Uh oh!

ghost commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

KirkMunro commented Aug 7, 2019 •

edited

Loading

KirkMunro commented Aug 22, 2019 •

edited

Loading