Skip to content
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

fix: scoop uninstall scoop #5685

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

hagaigold
Copy link
Contributor

@hagaigold hagaigold commented Oct 9, 2023

Description

scoop uninstall scoop is broken

Motivation and Context

Closes #5683

This is a "lazy" fixing of this issue by syncing the uninstall procedure commands from scoop-uninstall.ps1 to uninstall.ps1.
Please See the issue for more context, but a more robust way, is to refactor the code for a single function of uninstalling an app.

How Has This Been Tested?

Install 2 apps (one global) and then scoop uninstall scoop (see attached log)

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

test-5683.txt

@hagaigold
Copy link
Contributor Author

another test - scoop uninstall scoop -g

test-5683-uninstall-global.txt

@hagaigold hagaigold changed the title fix scoop uninstall scoop (#5683) fix scoop uninstall scoop Oct 9, 2023
@hagaigold hagaigold changed the title fix scoop uninstall scoop fix: scoop uninstall scoop Oct 9, 2023
@rashil2000
Copy link
Member

rashil2000 commented Oct 26, 2023

Do you think it would be better to invoke scoop-uninstall here, instead of invoking all the same functions? This will remove redundancy.

We can just invoke & "$PSScriptRoot\..\libexec\scoop-uninstall.ps1" $apps and pass in the array of all the apps, global and local separately.

@hagaigold
Copy link
Contributor Author

instead of invoking all the same functions?

I agree this is more robust way of doing things.
Because of my limited knowledge of this code base, the intention of this PR was just to fix the issue without too much effort.

Do you think it would be better to invoke scoop-uninstall

if you think this approach making sense (from scoop architecture perspective), I can give it another look.

@rashil2000
Copy link
Member

if you think this approach making sense (from scoop architecture perspective), I can give it another look.

Yes, please go ahead!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants