sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Fix dotnet test --blame parameters to validate incoming values, and update help

Open nohwnd opened this issue 2 years ago • 2 comments

nohwnd avatar Aug 03 '22 14:08 nohwnd

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@baronfel I've applied your suggestion to all of our bool parameters, but saw that some common parameters that have the same shape don't use it (--no-restore would be one example).

Maybe it is not a good idea to do this, if we consider that people might be using them for scripting. For example in powershell they would do:

function RunBuild {
  param ([switch] $NoRestore) 

  dotnet build --no-restore:$NoRestore
}

RunBuild -NoRestore -> dotnet build --no-restore:True
RunBuild -> dotnet build --no-restore:False

Making --no-restore zero arity leads to parsing error, and the value for that option is always True, because the parameter is always specified.

I think, this is very likely to happen in CI scripts for testing and so I am going to revert.

The [Switch] parameter type above in powershell code is using totally the same idea, it accepts zero or one argument, and implicitly casts from and to bool, to allow easy chaining of parameters from parent to child, or splatting/spreading parameters.

All of these are valid ways to call a function with a switch parameter:

RunBuild -NoRestore
RunBuild -NoRestore:$True

RunBuild
RunBuild -NoRestore:$False

RunBuild -NoRestore:$ParentNoRestore

$params = @{ NoRestore = $true }
RunBuld @params

nohwnd avatar Aug 10 '22 13:08 nohwnd