Powershell: Can we remove workflow code from PowerShell Core?

[KirkMunro]

ActionPreference.Suspend is not supported in PowerShell Core (it's only used for workflows, which are not in PowerShell Core). Today it just adds confusion to the code with checks to ensure that value isn't used where there doesn't need to be any. The checks should also be performed by a PSSA rule rather than in source code (similar to ActionPreference.Ignore, as was decided in #4348). Do we need to keep ActionPreference.Suspend checks around at this point or can we just remove the half-dozen checks related to it from the source code?

Further, if we remove these checks (for the same reason we removed the checks for ActionPreference.Ignore), do we need the ActionPreference.Suspend enumeration value kept around at all?

@SteveL-MSFT: For committee review.

[Update]
As per the committee review, all workflow code should be removed. Here is the list of items requiring removal:

• [x] workflow keyword
• [x] parallel keyword (used for parallel activities in workflow)
• [x] sequence keyword (used for sequential activities in workflow)
• [x] inlinescript activity block (used for activities outside of workflow)
• [x] -parallel parameter on foreach statement
• [x] Checkpoint-Workflow activity (command)
• [x] Suspend-Workflow activity (command)
• [x] ActionPreference.Suspend enumeration
• [x] plus all related classes/methods

[iSazonov]

If this does not work in any case it can be removed.
We could use our suggestion system to inform users.

[SteveL-MSFT]

@PowerShell/powershell-committee reviewed this and agree but should broaden this to reviewing all the code and remove all PSWorkflow code

[iSazonov]

There is a lot of such code. Maybe step-by-step will be suitable.

Should we be worried about remoting?

[KirkMunro]

Is there really a lot? Here is the list of things that need to be removed that I am aware of:

• [x] workflow keyword
• [x] parallel keyword (used for parallel activities in workflow)
• [x] sequence keyword (used for sequential activities in workflow)
• [x] inlinescript activity block (used for activities outside of workflow)
• [x] -parallel parameter on foreach statement
• [x] Checkpoint-Workflow activity (command)
• [x] Suspend-Workflow activity (command)
• [x] ActionPreference.Suspend enumeration
• [x] plus all related classes/methods

That's not that long of a list. In terms of doing it piecemeal or all at once, I removed all references to ActionPreference.Suspend as well as the enumeration value itself as part of PR #8205. It made great sense to do so, since a new ActionPreference enumeration value was being added in that PR (removing it now means the enumeration can be defined without specifically assigned values/gaps caused by removing it later); however, I don't feel removing the rest all at once is too daunting of a task.

[iSazonov]

@KirkMunro Thanks for the review! You could move your check list in the issue description and rename header.

I expected that almost everything was already removed there since we had already cleaned the code several times. Therefore, I was a little surprised that there is still relatively a lot of unnecessary code.

I believe we will make the cleanup step-by-step. Perhaps we will need some discussions because there is public APIs that I already catched in #9618.

[KirkMunro]

@iSazonov Good idea, and done.

[lzybkr]

There are good arguments for not removing keywords:

• You can provide better error messages (workflow not supported vs. command not found)
• It wouldn't be a breaking change to use the keyword in the future.

[msftrncs]

I also do not think that the keywords or parameter names should be moved entirely, and they are all already flagged as errors which is the right thing to do, in my opinion. The only downside to this is that means the workflow keyword cannot be used for a function name without using & (call).

[SteveL-MSFT]

Part of the reasoning by @PowerShell/powershell-committee to remove is to have the errors at parse time/compile time (for C# code) vs at runtime. There is no plan to ever bring back workflow. Instead, Jeffrey has a proposal for workflow-like state management that I'll publish a RFC for eventually.

[KirkMunro]

There is a public method on the Debugger class called GetWorkflowDebuggerFunctions that is not used internally and seems intended only for workflow. Given that is the case, can this (and the related const string function definitions) be removed?

[SteveL-MSFT]

@KirkMunro we've removed other workflow specific code that impacts public API, so now would be the time to remove it

[KirkMunro]

@SteveL-MSFT PowerShellGet internally uses the IsWorkflow property of FunctionDefinitionAst objects. How do you want that handled? I have that property removed from PR #10223 at the moment, which causes some Pester tests to fail because that property does not exist.

Futher, since PowerShellGet is managed independently of PowerShell itself, why are there PowerShellGet tests that use whatever latest version a user has installed in the PowerShell test suite?

[iSazonov]

It is not time critical to leave the property as dummy, then cleanup PowerShellGet, and then remove the property from ast.

[daxian-dbw]

I'm concerned about removing the workflow keyword. An error message like Workflow is not supported in PowerShell 6+ is better than The term 'workflow' is not recognized as the name of a cmdlet ....
/cc @SteveL-MSFT

[iSazonov]

I'm concerned about removing the workflow keyword.

@daxian-dbw Please look discussion in #10223

[msftbot[bot]]

This issue has been marked as fixed and has not had any activity for 1 day. It has been closed for housekeeping purposes.