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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(gitify): add preflight steps to reduce errors during brew upgrade #173061

Merged
merged 6 commits into from
May 14, 2024
Merged

fix(gitify): add preflight steps to reduce errors during brew upgrade #173061

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb65d9c
fix(gitify): attempt to reduce image errors during `brew upgrade`
setchy May 7, 2024
815ac2f
fix(gitify): add postflight script to reopen app
setchy May 7, 2024
02077cf
fix(gitify): add postflight script to reopen app
setchy May 7, 2024
bdfc863
fix(gitify): add preflight/postflight scripts
setchy May 7, 2024
38c51d6
fix(gitify): add preflight/postflight scripts
setchy May 7, 2024
8154bba
remove postflight script
setchy May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions Casks/g/gitify.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@

app "Gitify.app"

preflight do
retries ||= 3
ohai "Attempting to close Gitify.app to avoid unwanted user intervention" if retries >= 3
return unless system_command "/usr/bin/pkill", args: ["-f", "/Applications/Gitify.app"]
rescue RuntimeError
sleep 1
retry unless (retries -= 1).zero?
opoo "Unable to forcibly close Gitify.app"
end
Comment on lines +17 to +25
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with this in principle, but I think we would be better off adjusting the uninstall quit dsl to allow for this use case. If we add a preflight block here that closes the app, it sets a precedent to customise this in other casks. We try to avoid preflight/postflight blocks as much as possible and are looking to remove them. We should file an issue at github.com/homebrew/brew to track the idea of allowing certain apps to be quit forcibly first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Homebrew/brew#17247

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While discussion on DSL changes continues on Homebrew/brew#17247, would it be possible to move forward with this fix @bevanjkay.

Happy to pivot once the alternate route is available

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I guess it's the best option for now. I was hoping for a second opinion from another maintainer, I'll see if I can get one.


uninstall quit: [
"com.electron.gitify",
"com.electron.gitify.helper",
Expand Down