GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — 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

Conversation

@KirkMunro

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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
    • N/A
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
  • 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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov and mklement0 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
Kirk Munro (KirkMunro) added 2 commits August 7, 2019 20:10
@KirkMunro
remove deprecated ActionPreference.Suspend value
637ecbf
@KirkMunro
remove unused string
cd4b42c
@KirkMunro Kirk Munro (KirkMunro) mentioned this pull request Aug 7, 2019
Remove references to ActionPreference.Suspend MicrosoftDocs/PowerShell-Docs#4624
Closed
2 of 8 tasks
iSazonov
Ilya (iSazonov) approved these changes Aug 8, 2019
View reviewed changes

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM with one comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/CommandBase.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov
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
@iSazonov 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
@iSazonov Ilya (iSazonov) added this to the 7.0.0-preview.3 milestone Aug 8, 2019
@iSazonov Ilya (iSazonov) self-assigned this Aug 8, 2019
@iSazonov Ilya (iSazonov) added the Breaking-Change breaking change that may affect users label Aug 8, 2019
PaulHigin
Paul Higinbotham (PaulHigin) approved these changes Aug 8, 2019
View reviewed changes

@PaulHigin Paul Higinbotham (PaulHigin) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@KirkMunro Kirk Munro (KirkMunro) mentioned this pull request Aug 8, 2019
Code cleanup: bulk removal of deprecated workflow code #10223
Closed
8 of 15 tasks
@SteveL-MSFT Steve Lee (SteveL-MSFT) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Aug 19, 2019
@TravisEz13 Travis Plunk (TravisEz13) modified the milestones: 7.0.0-preview.3, 7.0.0-preview.4 Aug 19, 2019
@SteveL-MSFT

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

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

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

Copy link
Copy Markdown
Member

@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.
vexx32 and iSazonov reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT 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
@iSazonov 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
@KirkMunro
Merge branch 'master' into deprecate-actionpreference-suspend
c53b9eb
@KirkMunro
put enum value back, updated plumbing and tests
1d84d26
@KirkMunro

Kirk Munro (KirkMunro) commented Aug 22, 2019

Copy link
Copy Markdown
Contributor Author

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

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

I'll also update the OP accordingly.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@KirkMunro

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@KirkMunro
CodeFactor changes
008d97b
@KirkMunro 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
PoshChan-Bot (PoshChan) reviewed Aug 22, 2019
View reviewed changes
Comment thread src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
PoshChan
PoshChan-Bot (PoshChan) reviewed Aug 22, 2019
View reviewed changes
Comment thread src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@KirkMunro
remove global qualifier added during testing
d68b07b
@KirkMunro

Kirk Munro (KirkMunro) commented Aug 26, 2019

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Aug 27, 2019

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
PaulHigin
Paul Higinbotham (PaulHigin) suggested changes Aug 29, 2019
View reviewed changes
Comment thread src/System.Management.Automation/engine/ExecutionContext.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/System.Management.Automation/engine/MshCommandRuntime.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/System.Management.Automation/engine/MshCommandRuntime.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ghost 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
@KirkMunro
code review changes
5eb9658
PaulHigin
Paul Higinbotham (PaulHigin) approved these changes Aug 29, 2019
View reviewed changes
@KirkMunro

Kirk Munro (KirkMunro) commented Sep 4, 2019

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Sep 5, 2019

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daxian-dbw
Dongbo Wang (daxian-dbw) reviewed Sep 5, 2019
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/System.Management.Automation/engine/ExecutionContext.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw 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
@KirkMunro
Merge branch 'master' into deprecate-actionpreference-suspend
1962ea8
@KirkMunro
remove ActionPreference.Ignore restriction
7b0c053
@KirkMunro Kirk Munro (KirkMunro) changed the title Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state. Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables Sep 6, 2019
Kirk Munro (KirkMunro) added a commit to KirkMunro/PowerShell that referenced this pull request Sep 6, 2019
@KirkMunro
move ActionPreference.Ignore fix to PowerShell#10317
9fda03f
@KirkMunro Kirk Munro (KirkMunro) mentioned this pull request Sep 6, 2019
Add VerboseAction, DebugAction, ProgressAction, VerboseVariable, DebugVariable, and ProgressVariable common parameters #10238
Closed
10 of 14 tasks
@iSazonov 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
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Sep 6, 2019
View reviewed changes
@daxian-dbw 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
@iSazonov 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
@iSazonov
Ilya (iSazonov) merged commit 139cd94 into PowerShell:master Sep 7, 2019
@KirkMunro
Kirk Munro (KirkMunro) deleted the deprecate-actionpreference-suspend branch September 9, 2019 17:31
Travis Plunk (TravisEz13) pushed a commit to TravisEz13/PowerShell that referenced this pull request Sep 14, 2019
@KirkMunro @TravisEz13
Transition ActionPreference.Suspend enumeration value into a non-supp…
61a10ce 
…orted, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables (PowerShell#10317)
@ghost

Deleted user (ghost) commented Sep 19, 2019

Copy link
Copy Markdown

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

Handy links:

  • Release Notes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw 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
@SeeminglyScience 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
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@KirkMunro @iSazonov
Transition ActionPreference.Suspend enumeration value into a non-supp…
fe8744d 
…orted, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables (PowerShell#10317)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
+2 more reviewers
@PoshChan PoshChan-Bot (PoshChan) PoshChan left review comments
@PaulHigin Paul Higinbotham (PaulHigin) PaulHigin approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

7.0.0-preview.4

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@KirkMunro @SteveL-MSFT @iSazonov @daxian-dbw @PaulHigin @PoshChan @TravisEz13
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.