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

Defer :script uninstall, add :early_script #2391

Merged
merged 1 commit into from
Jan 28, 2014

Conversation

rolandwalker
Copy link
Contributor

fixes #2254

Previously (#2024), I avoided moving :script to the most sensible place in the order, in part to avoid breaking something.

This PR proposes to re-order :script where it belongs, and add uninstall key :early_script to preserve existing functionality exactly. If any Cask breaks because of re-ordering :script, that Cask can adopt :early_script instead. If no Casks adopt :early_script we can consider it transitional clutter and remove it later on.

:early_script is added to docs, but deprecated. Code comments are added explaining the reasoning behind uninstall order.

@rolandwalker
Copy link
Contributor Author

merge conflicts resolved

@@ -71,6 +75,7 @@ def manually_uninstall(uninstall_options)
end
end

# :quit must come before :kext so the kext will not be in use by the app
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call on the extra comments to help us understand the order 👌

@phinze
Copy link
Contributor

phinze commented Jan 28, 2014

+1 to logic and implementation. Merging

phinze added a commit that referenced this pull request Jan 28, 2014
Defer :script uninstall, add :early_script
@phinze phinze merged commit 3179a55 into Homebrew:master Jan 28, 2014
@rolandwalker rolandwalker deleted the uninstall_script_order branch January 28, 2014 15:02
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run :quit before :script
2 participants