fix #156 - Start-RSJob - be able to import private functions

[copdips]

add a new param : FunctionFilesToLoad
and some code formatting.

[proxb]

This will be a breaking change for those who have been using this parameter. I would recommend adding an alias on FunctionsToImport called FunctionsToLoad.

[copdips]

hi,

I just cloned the master branch of the repo, if we take a look at the line 178 of :
https://github.com/proxb/PoshRSJob/blob/master/PoshRSJob/Public/Start-RSJob.ps1

The real param name is FunctionsToLoad but not FunctionsToImport, I just correct it in the help file to be the same as declared in the function param block.

And the new param that I added is : FunctionFilesToLoad

[proxb]

Gotcha. That makes sense now. Thanks for the additional information!

[proxb]

This will be a breaking change for those who have been using this parameter. I would recommend adding an alias on FunctionsToImport called FunctionsToLoad. Other than that, I think everything looks good.

[MVKozlov]

Seems because of using AST and [pscustomobject] these changes doesnt support PSv2

[copdips]

@MVKozlov,

Well, yes, AST is used which is not supported by PSv2.

But if we keep the compatibility for V2, we will lost many powerful cmdlets/functionalities provided by the later versions. V2 might be considered as deprecated now.

Nevertheless, I'll push a new commit to let it be compatible with V2 by using the FindFunction.ps1

Reg. the V2 support lifecycle, please refer to :
https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/

https://twitter.com/jsnover/status/889153919527419905

[copdips]

@proxb @MVKozlov ,

Hi,
Please review.

• rebase + added a test case + removed some trailing whitespaces
• compatible with Powershell v2+.

[MVKozlov]

Looks good, but I think will be better if Start-RSJob.ps1's

275   +                                $functionDefinition = $function.Body.Trim().Trim("{}")  
276   +                            }  
277   +                            Default {  
278   +                                $functionDefinition = $function.Body.Extent.Text.Trim("{}")  

will be inside GetFunсtionByFile

btw, GetFun --c-- tionByFile ;-)

[copdips]

@MVKozlov
Thx for the review, GetFunсtionByFile returns functions as per its name, if I also add the function->definition part inside, it doesn't make sense to me.
Maybe I can create a new private lib called GetFunctionDefinitionByFunction ?

Or I add the part inside, and rename the file GetFunсtionByFile.ps1 to GetFunсtionDefinitionByFile.ps1 ?

[MVKozlov]

I think it doesn't matter (almost) how named internal function, but I prefer new functional mimic existing implementation in Start-RSJob and now it looks like
$var = getinfo (v2 vs v5 difference here); use $var
and yours $var = getinfo (v2 vs v5 difference here); if ($PSVersionTable.PSVersion.Major ...) use $var.method1 else use $var.method2
i.e. you repeat code Switch ($PSVersionTable.PSVersion.Major) { ... } twice

Personally me strongly dislike if $PSVersionTable.PSVersion.Major ... around $PSBoundParameters['VariablesToImport'] and want to move it to privates, especially because of #162.
But I wait for @proxb to stabilize current version of code.

[copdips]

I agree with you, all the (v2 vs v5) should be removed from Start-RsJob to some privates.
We should also do some refacto of Start-RsJob, the script body is too big. Personally, I prefer many tiny functions to a big one.

[copdips]

Hi @proxb @MVKozlov ,

Please review.

1. As per MVKozlov's advise, I moved the powershell version check part to the private function GetFunctionDefinitionByFunction.ps1.
2. fix typo GetFun --c-- tionByFile

[MVKozlov]

I like it, wait for @proxb :)

[proxb]

I'm so late to this. As far as I can tell, it looks good to me. I'll get this approved by the end of the week.

[proxb]

Late to get this done, but now it's done. :)