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

Acceptable cases for :target #6408

Closed
wants to merge 18 commits into from
Closed

Acceptable cases for :target #6408

wants to merge 18 commits into from

Conversation

vitorgalvao
Copy link
Member

@caskroom/maintainers Stemming from the conversation in #6375, this is the proposal (with appropriate changes) for the rules pertaining to the use of :target. Its use with app has been tightened, while its use with binary has remained liberal in comparison (if it should remain so is also something that should be agreed upon).

Regarding apps, I’ve tried to keep the message consistent between them, and found three main reasons for renaming with :target:

  • For clarity: app name is inconsistent with its branding.
  • For consistency: app name is different in the Finder and in a shell.
  • To avoid conflict.

@rolandwalker
Copy link
Contributor

+1. Thanks for drilling into this. I reviewed and checked for a few cases which are known to me such as LogMeIn.

The imagemin.rb quoting issue is my fault from one of the recent mass updates.

BTW some of the Chinese-language apps also have an inconsistency between app bundle name as visible in Finder, and app name as visible in the menu.

@vitorgalvao
Copy link
Member Author

Do you know which ones? The ones I’ve checked seemed fine.

baiducloud
baiduinput
baidumusic
powerword
qiyimedia
sogouinput
thunder
youdao

@alebcay
Copy link
Member

alebcay commented Oct 1, 2014

I would check thunder.

@vitorgalvao
Copy link
Member Author

Also clean. Adding it to the previous list, so we can keep track.

@rolandwalker
Copy link
Contributor

Sorry, I can't remember. It might have been fixed upstream.

rolandwalker added a commit that referenced this pull request Oct 2, 2014
* a repaired version of PR #6408
  * added :target accepted cases, to language reference
  * fixed quote weirdness in imagemin
  * clarified reason to :target rename scala-ide
  * clarified reason to :target rename music-manager
  * clarified reason to :target rename logmein-client
  * clarified reason to :target rename imagemin
  * clarified reason to :target rename flash-player-debugger
  * clarified reason to :target rename devonthink-pro-office
  * clarified reason to :target rename bitcoin-core
  * don't rename gingr
  * renamed youtrack-workflow-editor in accordance with the naming conventions
  * don't :target rename youtrack-workflow-editor
  * don't rename textsoap
  * don't rename mini-metro
  * don't rename liquifile
  * renamed firewall-builder to fwbuilder, as per the naming conventions
  * don't :target rename firewall-builder
  * don't rename wings3d
@rolandwalker
Copy link
Contributor

Repaired the conflicts and merged manually. Sorry if you weren't ready; I wanted to get the imagemin fix in.

@vitorgalvao
Copy link
Member Author

It’s fine, it was ready; was just waiting to see if there were any objections. Thank you for taking care of the conflicts.

@vitorgalvao vitorgalvao deleted the remove-targets branch October 2, 2014 15:00
@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.

3 participants