Repository navigation
Can we remove workflow code from PowerShell Core? #9570
Description
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?
Steve Lee (@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:
- workflow keyword
- parallel keyword (used for parallel activities in workflow)
- sequence keyword (used for sequential activities in workflow)
- inlinescript activity block (used for activities outside of workflow)
- -parallel parameter on foreach statement
- Checkpoint-Workflow activity (command)
- Suspend-Workflow activity (command)
- ActionPreference.Suspend enumeration
- plus all related classes/methods
Activity
iSazonov commentedon May 11, 2019
If this does not work in any case it can be removed.
We could use our suggestion system to inform users.
SteveL-MSFT commentedon May 15, 2019
@PowerShell/powershell-committee reviewed this and agree but should broaden this to reviewing all the code and remove all PSWorkflow code
iSazonov commentedon May 16, 2019
There is a lot of such code. Maybe step-by-step will be suitable.
Should we be worried about remoting?
KirkMunro commentedon May 16, 2019
Is there really a lot? Here is the list of things that need to be removed that I am aware of:
That's not that long of a list. In terms of doing it piecemeal or all at once, I removed all references to
ActionPreference.Suspendas well as the enumeration value itself as part of PR #8205. It made great sense to do so, since a newActionPreferenceenumeration 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.removed ActionPreference.Suspend (per PowerShell#9570)
iSazonov commentedon May 16, 2019
Kirk Munro (@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.
[-]Can we remove checks for `ActionPreference.Suspend` from PowerShell Core?[/-][+]Can we remove workflow code from PowerShell Core?[/+]KirkMunro commentedon May 16, 2019
Ilya (@iSazonov) Good idea, and done.
lzybkr commentedon May 16, 2019
There are good arguments for not removing keywords:
msftrncs commentedon May 16, 2019
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
workflowkeyword cannot be used for a function name without using&(call).SteveL-MSFT commentedon May 18, 2019
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 commentedon Jul 24, 2019
There is a public method on the
Debuggerclass calledGetWorkflowDebuggerFunctionsthat 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 commentedon Jul 24, 2019
Kirk Munro (@KirkMunro) we've removed other workflow specific code that impacts public API, so now would be the time to remove it
KirkMunro commentedon Jul 25, 2019
Steve Lee (@SteveL-MSFT) PowerShellGet internally uses the
IsWorkflowproperty ofFunctionDefinitionAstobjects. 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 commentedon Jul 25, 2019
It is not time critical to leave the property as dummy, then cleanup PowerShellGet, and then remove the property from ast.
daxian-dbw commentedon Jul 25, 2019
I'm concerned about removing the
workflowkeyword. An error message likeWorkflow is not supported in PowerShell 6+is better thanThe term 'workflow' is not recognized as the name of a cmdlet ..../cc Steve Lee (@SteveL-MSFT)
iSazonov commentedon Jul 26, 2019
Dongbo Wang (@daxian-dbw) Please look discussion in #10223
mavaddat commentedon Jun 1, 2023
It would be cool if PowerShell Core provided modularity on the workflow engine choice so that endusers could opt to choose their workflow adapter assembly. I have floated this idea in the
UiPath/CoreWFdiscussion.