Sycnex / Windows10Debloater

Script to remove Windows 10 bloatware.
MIT License
18.19k stars 2.05k forks source link

Inconsistent and faulty use of service functions: DiagTrack, Diag WAPPush/CheckDMWService #141

Open Marsh390 opened 5 years ago

Marsh390 commented 5 years ago
  1. Windows10Debloater.ps1/Protect-Privacy has the disable processing for DiagTrack service, but there is no equivalent for the dmwappushservice service shouldn’t it?
  2. Windows10DebloaterGUI.ps1/Protect-Privacy has no DiagTrack or dmwappushservice disable processing – shouldn’t it have both?
  3. Windows10DebloaterGUI.ps1/Revert-Changes has DiagTrack and dmwappushservice enable processing-which seems correct.
  4. In the “Main” portion of Windows10Debloater.ps1 (i.e. lines 705+) a. The ‘Yes’ portion (lines 737+) has a DisableDiagTrack and DisableWapPush function (which you would expect), but the seem to be non-existent and if you set the $ErrorActionPreference = ‘stop’ just before this and then reset (if you want) just after this you will see the error that they do not exist. b. CheckDMWService (which does exist and starts the service) seems to be something you would want in ‘revert-changes’ not ‘protect-privacy’. (plus it would undo DisableWapPush if that existed). c. [Side Note]: in my version of this I have the start/stop (or enable/disable) service functions combined into one routine and just pass a true/false flag to one routine that does all four functions (enable/disable for both services). You seem to have a new InstallService so maybe that can be handled by that too. d. [Side Note]: Since the Yes/No portion of the Everythingorspecific Debloat prompt (lines 731 to 800) does the same processing except for DebloatAll vs DebloatBlacklist and Remove3DObjects (why doesn’t DebloatBlacklist have this?) why not put the common processing after the yes/no processing to declutter the code. e. [Side Note]: Since you have a lot of Write-Host messages and Start-Sleep whny not put these within the functions and declutter the code-your functions names are well described enough to know what they do.
  5. [Side Note]: If the functions in Windows10Debloater.ps1 were put in a separate file that the GUI version could access then there would be no way to be out-of-sync with each other- I know, ‘cause I have a working version just that way--the GUI script is minimal and way easier to maintain.
albvar commented 5 years ago

+1 Additionally leverage PSSCriptAnalyzer to detect these issues early. You have a ton of trailing white spaces within the code, can be replaced with this regex [ \t]+$ Adding reference for AvoidTrailingWhiteSpace rule https://github.com/PowerShell/PSScriptAnalyzer/blob/development/RuleDocumentation/AvoidTrailingWhitespace.md