-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(spotify) Remove Windows Store version #2263
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,24 @@ $arguments = @{ | |
validExitCodes = @(0, 1641, 3010) | ||
} | ||
|
||
# check we can use Get-AppxPackage | ||
if (Get-Command 'Get-AppxPackage' -ErrorAction SilentlyContinue) { | ||
# there is likely going to be two packages returned for x86 and x64. | ||
# we don't care about the architecture, just the version and they will both be the same. | ||
$allAppxPackages = Get-AppxPackage | ||
$installedAppXPackage = @($allAppxPackages | Where-Object -Property Name -eq 'SpotifyAB.SpotifyMusic') | ||
if ($installedAppXPackage.Count -gt 0) { | ||
if ($env:ChocolateyForce) { | ||
#when you remove a package, you don't remove it per architecture. You just remove it for all architectures. | ||
Write-Warning 'Attempting to remove Spotify installed from the Microsoft Store.' | ||
Remove-AppxPackage -Package $installedAppXPackage[0].PackageFullName | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, we should not remove the Microsoft Store application in this package. It is still two different applications, which goes against having one software for each package. IMO, I think we should throw an exception instead, no matter what, possibly with details on how the user can uninstall the application manually or add a new package that only has the purpose of removing the Store version of Spotify (that we possibly can depend on in this package). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A package parameter? Warn them but if they add the package parameter it also uninstalls. Seems like the best of both worlds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm. Add it behind a package parameter 🤔. Yeah, could work as well. Let's go with that instead. I still do think a separate package could be beneficial though, but we can go with the parameter for now. |
||
} | ||
else { | ||
throw "Cannot install the Spotify package because the Microsft Store version is installed. Please uninstall this manually or add the '--force' option to the command line." | ||
} | ||
} | ||
} | ||
|
||
# Download the installer | ||
$arguments['file'] = Get-ChocolateyWebFile @arguments | ||
|
||
|
@@ -24,10 +42,10 @@ schtasks.exe /Delete /TN $arguments['packageName'] /F | |
# Wait for Spotify to start, then kill it | ||
$done = $false | ||
do { | ||
if (Get-Process Spotify -ErrorAction SilentlyContinue) { | ||
Stop-Process -name Spotify | ||
$done = $true | ||
} | ||
if (Get-Process Spotify -ErrorAction SilentlyContinue) { | ||
Stop-Process -name Spotify | ||
$done = $true | ||
} | ||
|
||
Start-Sleep -s 10 | ||
Start-Sleep -s 10 | ||
} until ($done) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the indentation used on the lines changed to follow the editor config file.
The editor config file specifies that indentations should only be two spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is inconsistent. I'll update the file to match 2 spaces.