Add ShouldProcess to New-FileCatalog and Test-FileCatalog - #3074
Conversation
Close PowerShell#3068 Add support `-WhatIf` and `-Confirm` to `New-FileCatalog` and add a test. `Test-FileCatalog` has a common code base with `New-FileCatalog` so it automatically get the same. I believe that adding a separate test in this case doesn't make sense.
|
|
||
| Collection<string> paths = new Collection<string>(); | ||
|
|
||
| bool _ShouldProcess = false; |
There was a problem hiding this comment.
You don't need this boolean. You can use (paths.Count > 0) instead of (_ShouldProcess) to see if processing should continue.
There was a problem hiding this comment.
Paul's suggestion is good, but I'm fine with a local variable as well - but the local variable should not use the convention for fields - it should be shouldProcess.
There was a problem hiding this comment.
Paul Higinbotham (@PaulHigin) Good catch!
Jason Shirk (@lzybkr) If we take into consideration that cmdlets can work with large directories, then maybe we will remove the local variable and add a comment 'paths.Count > 0 is true only if ShouldProcess allow'?
There was a problem hiding this comment.
I removed _ShouldProcess.
There was a problem hiding this comment.
Paul Higinbotham (@PaulHigin) I think your comment has been addressed
| try | ||
| { | ||
| $null = New-FileCatalog -Path $sourcePath -CatalogFilePath $catalogPath -WhatIf | ||
| $result = Test-Path -Path ($catalogPath + "\catalog.cat") |
There was a problem hiding this comment.
I would introduce a variable $catalogFile or something like that and use the variable when trying to delete the file.
I would also probably not call Remove-File if Test-Path says the file isn't there.
Remove _ShouldProcess Add var in test
…#3074) * Add ShouldProcess to New-FileCatalog and Test-FileCatalog Close PowerShell#3068 Add support `-WhatIf` and `-Confirm` to `New-FileCatalog` and add a test. `Test-FileCatalog` has a common code base with `New-FileCatalog` so it automatically get the same. I believe that adding a separate test in this case doesn't make sense. * Fiz after code review Remove _ShouldProcess Add var in test
Close #3068
Add support
-WhatIfand-ConfirmtoNew-FileCatalogand add atest.
Test-FileCataloghas a common code base withNew-FileCatalogso itautomatically get the same. I believe that adding a separate test in
this case doesn't make sense.