PoshRSJob icon indicating copy to clipboard operation
PoshRSJob copied to clipboard

pre-PR discussion. Breaking changes in Start-RSJob parameter handling

Open MVKozlov opened this issue 7 years ago • 3 comments

Before I make Pull request with my changes I want to discuss it with community.

NewParam branch of my fork contains several fixes and changes that lead to behaviour change of Start-RSJob function

here is the main article https://github.com/MVKozlov/PoshRSJob/wiki/NewParam-branch-changes

Danger: long text inside :)

Small summary: Start-RSJob can change the way it deals with parametrized scriptblocks, somethimes it stops add $_ into param(), sometimes contrariwise

I think that these changes are good, but you can call them evil :)

Tell me what you think !

MVKozlov avatar May 12 '17 15:05 MVKozlov

The description is very long and complex. I don't think it affects me because I always declare param() in my scriptblock.

I always assumed -ArgumentList should not work with Pipeline. There are no good reasons to allow both at the same time. People should use one or the other.

My goal is for PoshRSJob to become simpler. The code should be shorter. It has too much functionality that isn't used and causes lots of bugs.

Does this do that?

codykonior avatar May 16 '17 08:05 codykonior

Yes, the new code is shorter and simpler, it combine Start-RSJob and $data | Start-RSJob cases in one code branch. It also remove bug when first parameter added as array even it not array

new Start-RSJob never insert $_ if called like Start-RSJob -ArgumentList $a, $b, $c as it do now.

it combines two cases in one when $data | Foreach-Object { Start-RSJob $scriptblock } works with data and arguments like $data | Start-RSJob $scriptblock

but I can't agree with

There are no good reasons to allow both at the same time. People should use one or the other.

people expects that $data | Start-RSJob {param($par1) } -ArgumentList $arg1 will work so $par1 = $arg1 (and $data go to $_)

and $data | Start-RSJob {param($par1,$par2) } -ArgumentList $arg1,$arg2 also get $par1 = $arg1, $par2=$arg2 ...

but what we should do if user call it like $data | Start-RSJob{param($par1,$par2) } -ArgumenLlist $arg1 or $data | Start-RSJob {param($par1) } -ArgumentList $arg1, $arg2, $arg3 ? ?

And here comes the moment when nuances occur ...

May be we should just throw ?? May be we should always convert $InputObject into serie of $_ and directly match parameters to args

So I created this topic to get answers.

Anyway, I think new code work with parameters more transparent way than old And it can be modified as we like in a simpler way.

But how we like ? :)

MVKozlov avatar May 16 '17 11:05 MVKozlov

@codykonior I rewritten long text into other long text ...

in addition I published test script. With it you can clearly see that you too can be affected by these changes if you use Start-RSJob {param($params) } -ArgumentList $args scenario

And in latest commit I included code variants that can mimic current behaviour for different scenarios

MVKozlov avatar May 17 '17 10:05 MVKozlov