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.

Sign up for GitHub

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

Jump to bottom

fix(scoop-uninstall): run pre_uninstall before testing running processes #4962

Merged
merged 5 commits into from
Jun 3, 2022
Merged

fix(scoop-uninstall): run pre_uninstall before testing running processes #4962

merged 5 commits into from
Jun 3, 2022

Conversation

beer-psi
Copy link
Contributor

@beer-psi beer-psi commented Jun 2, 2022

Description

Execute the pre_uninstall hook before other uninstallation preparation work, namely test_running_process

Motivation and Context

Closes #4961

Motivations

  • pre_uninstall script should handle teardown work such as stopping services, so that the uninstallation process isn't blocked by running processes.
  • Matches symmetry with post_uninstall, which is run at the very end of the uninstallation process.

How Has This Been Tested?

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.

@beer-psi beer-psi mentioned this pull request Jun 2, 2022
Closed
@beer-psi
Copy link
Contributor Author

beer-psi commented Jun 2, 2022

One question: Is a CHANGELOG.md entry required, considering pre_ and post_uninstall hooks haven't been officially released?

@niheaven
Copy link
Member

niheaven commented Jun 2, 2022

Add (#4962)[https://github.com/ScoopInstaller/Scoop/issues/4962] after the original PR number, see other multi–PRs CHANGELOG entries.

@beer-psi
Copy link
Contributor Author

beer-psi commented Jun 2, 2022

Alright, I've done that

niheaven
niheaven reviewed Jun 2, 2022
View reviewed changes
beer-psi and others added 2 commits June 3, 2022 05:52
@beer-psi @niheaven
Update CHANGELOG.md
11025a6 
Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
@beer-psi
fix: fetch manifest before invoking pre_uninstall hook
3d9edc6
@beer-psi beer-psi requested a review from chawyehsu June 2, 2022 23:11
@chawyehsu chawyehsu merged commit 78c1bc4 into ScoopInstaller:develop Jun 3, 2022
@niheaven niheaven mentioned this pull request Jun 3, 2022
Merged
@niheaven niheaven mentioned this pull request Jun 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niheaven niheaven niheaven left review comments

@chawyehsu chawyehsu chawyehsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beer-psi @niheaven @chawyehsu