search

MSEndpointMgr / ConfigMgr

Microsoft Endpoint Configuration Manager scripts and tools
633 stars 282 forks source link

Update Invoke-RemoveBuiltinApps.ps1 #189

Closed baardhermansen closed 4 years ago

baardhermansen commented 4 years ago

Moved $null to the left side of the operator. Corrected warning message to use variable of file name. Corrected catch regarding package name.

NickolajA commented 4 years ago

Appreciate the update, however I do not think some of the changes makes sense, especially why moving the $null around. I'll update with the $FileName variable though, which does makes sense.

baardhermansen commented 4 years ago

Null on left follows best practice, see https://github.com/PowerShell/PSScriptAnalyzer/blob/development/RuleDocumentation/PossibleIncorrectComparisonWithNull.md.

Changing the variables inside the catch is because of the reason for the catch; $AppPackageFullname or $AppProvisioning is empty, hence there's no name to write to the logs. Unless I'm a complete dimwit and fail to understand how stuff works :P

NickolajA commented 4 years ago

I'm the dimwit that didn't see that about the catch statements, fixed (thanks!). However I'm not liking the $null before the variable, even if it's "best practice". It just doesn't read right in my head.

baardhermansen commented 4 years ago

Whohoo, i did something correct! :) I know how you feel about the re-positioning of the $null, but when i started reading about it, it's something that's been promoted for a number of years. We're just the ones who've been doing it wrong for too long ;)