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

music-manager.rb: do not move prefpane #21659

Merged
merged 2 commits into from
Jun 3, 2016
Merged

Conversation

vitorgalvao
Copy link
Member

Editing an existing cask

  • Commit message includes cask’s name (and new version, if applicable).
  • brew cask audit --download {{cask_file}} is error-free.
  • brew cask style --fix {{cask_file}} left no offenses.

Why would we be taking it? Presumably the app will take care of it.

@tjanson
Copy link
Contributor

tjanson commented Jun 3, 2016

See #20405:

The app will copy the pref pane itself on first run (if it does not already exist), but then brew uninstall will not remove it.
By letting brew symlink the pref pane, it uninstalls properly.

@tsparber
Copy link
Contributor

tsparber commented Jun 3, 2016

@tjanson Would a uninstall delete: '~/Library/Preference Panes/MusicManager.prefPane' also work in this case?

@vitorgalvao
Copy link
Member Author

@tjanson I was going to suggest what @tsparber just mentioned. As to why I’m removing this, see #21657 (comment). Since we now move instead of linking, this current cask essentially modifies the app (since it removes the prefpane from inside) which isn’t good at all.

@vitorgalvao vitorgalvao merged commit 6ca63cc into master Jun 3, 2016
@vitorgalvao vitorgalvao deleted the music-manager-prefpane branch June 3, 2016 22:44
@@ -10,4 +10,6 @@
# Renamed for consistency: app name is different in the Finder and in a shell.
# Original discussion: https://github.com/caskroom/homebrew-cask/pull/4282
app 'MusicManager.app', target: 'Music Manager.app'

uninstall delete: '~/Library/Preference Panes/MusicManager.prefPane'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be ~/Library/PreferencePanes/… (no space), right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just used the path from man brew-cask without checking. Turns out this needed some fixing (#21670).

@tjanson
Copy link
Contributor

tjanson commented Jun 3, 2016

Ah, cool to see that the move to move has been made. 👍
Removing the pane during uninstall works for me, but the path is "~/Library/PreferencePanes/MusicManager.prefPane".

vitorgalvao pushed a commit that referenced this pull request Jun 3, 2016
chizmw pushed a commit to chizmw/homebrew-cask that referenced this pull request Jun 15, 2016
* music-manager.rb: do not move prefpane

* remove prefpane on uninstall
chizmw pushed a commit to chizmw/homebrew-cask that referenced this pull request Jun 15, 2016
@miccal miccal removed the cask label Dec 23, 2016
@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.

4 participants