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

Add Sound Blaster Command App #83746

Merged
merged 7 commits into from
Jun 5, 2020
Merged

Add Sound Blaster Command App #83746

merged 7 commits into from
Jun 5, 2020

Conversation

iMonZ
Copy link
Contributor

@iMonZ iMonZ commented Jun 4, 2020

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

After making all changes to a cask, verify:

Additionally, if adding a new cask:

  • Named the cask according to the token reference.
  • brew cask install {{cask_file}} worked successfully.
  • brew cask uninstall {{cask_file}} worked successfully.
  • Checked the cask was not already refused.
  • Checked the cask is submitted to the correct repo.

@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 4, 2020

Maybe I need some help with the uninstaller script.
But this Script already included worked too but need a User input
(This Software uses a tool with an UI for Uninstalling)

Copy link
Contributor Author

@iMonZ iMonZ left a comment

Choose a reason for hiding this comment

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

Fixed an Empty Folder

Copy link
Contributor Author

@iMonZ iMonZ left a comment

Choose a reason for hiding this comment

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

Sorry

@miccal
Copy link
Member

miccal commented Jun 4, 2020

Please run brew cask style --fix sb-command as asked in the template:

==> brew cask style sb-command
== /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Casks/sb-command.rb ==
C:  8:  1: stanza groups should be separated by a single empty line
C:  9:  1: stanza groups should be separated by a single empty line
C: 10:  1: Trailing whitespace detected.
C: 11: 29: Use 2 spaces for indentation in a hash, relative to the position of the opening brace.
C: 11: 29: Align the keys and values of a hash literal if they span more than one line.
C: 11: 39: Space missing after colon.
C: 14: 27: Indent the right brace the same as the left brace.
C: 15:  9: Align the keys and values of a hash literal if they span more than one line.
C: 16: 16: Use 2 spaces for indentation in an array, relative to the position of the opening bracket.
C: 17: 14: Indent the right bracket the same as the left bracket.

Done!
I have used the Command: brew cask style Casks/sb-command.rb [--fix] from https://github.com/Homebrew/homebrew-cask/blob/master/doc/development/adding_a_cask.md

But this command doesn't fixed the offences
Copy link
Contributor Author

@iMonZ iMonZ left a comment

Choose a reason for hiding this comment

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

Done!
I have used the Command: brew cask style Casks/sb-command.rb [--fix] from https://github.com/Homebrew/homebrew-cask/blob/master/doc/development/adding_a_cask.md

But this command doesn't fixed the offences

@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 4, 2020

It would be great if you would help me with the Uninstaller.
Travis cannot test it cause of the need of User Input

Copy link
Contributor

@ran-dall ran-dall left a comment

Choose a reason for hiding this comment

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

@iMonZ Sometimes when you execute a pkg you don't need a zap stanza because pkg requires uninstall, however, I've made the zap stanza (just in case) which will ensure everything gets removed.

Also, if the name is Sound Blaster Command, then the .rb needs to be sound-blaster-command.rb

Casks/sb-command.rb Outdated Show resolved Hide resolved
@ran-dall ran-dall added the awaiting user reply Issue needs response from a user. label Jun 4, 2020
@iMonZ iMonZ changed the title Add SoundBlaster Command App Add SB Command App Jun 4, 2020
Hey thank you!

The name of the app after installing and on the Official Website is SB command!
The tool from homebrew suggested the name for the .rb file as SB-command.
So I need really to change this or let this at SB-command what is also better for the user cause of the same name on the website

Co-authored-by: Randall <17261190+ran-dall@users.noreply.github.com>
@iMonZ iMonZ requested a review from ran-dall June 4, 2020 23:41
@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 4, 2020

Hey thank you!

The name of the app after installing and on the Official Website is SB command!
The tool from homebrew suggested the name for the .rb file as SB-command.
So I need really to change this or let this at SB-command what is also better for the user cause of the same name on the website

@ran-dall
Copy link
Contributor

ran-dall commented Jun 5, 2020

@iMonZ The main reason I'd change it to Sound Blaster Command is also because when a user runs brew search they would have to enter sb command or sb-command to see the cask. IMO sb-command is a bit ambiguous, so I'd personally prefer sound-blaster-command.

HOWEVER, I don't really have a problem with keeping it with sb-command if you want. If that's the case though, I'd change the name stanza to SB Command. Which is technically the name of the app that gets installed.

@iMonZ iMonZ changed the title Add SB Command App Add Sound Blaster Command App Jun 5, 2020
@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 5, 2020

Okay Right I’m changing it and run the style fix then you can merge

@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 5, 2020

Done!
It can be merged now.

Copy link
Contributor

@ran-dall ran-dall left a comment

Choose a reason for hiding this comment

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

@iMonZ LGTM. Good work!

@ran-dall ran-dall removed the awaiting user reply Issue needs response from a user. label Jun 5, 2020
@ran-dall ran-dall merged commit 31c6f7a into Homebrew:master Jun 5, 2020
@iMonZ iMonZ deleted the sb-command branch June 5, 2020 20:01
@iMonZ
Copy link
Contributor Author

iMonZ commented Jun 5, 2020

Good work from your side!
Thank you for your work

@iMonZ iMonZ mentioned this pull request Jul 24, 2020
9 tasks
@vitorgalvao
Copy link
Member

vitorgalvao commented Aug 30, 2020

Moving to homebrew/cask-drivers. It definitely belongs there, as it’s useless without a peripheral:

Also required a kext caveat (noticed when installing on a VM).

Because that repo requires the company’s name as a prefix, the correct name of the app sb-command is no longer ambiguous. Leaving name with Sound Blaster for searchability.

It has almost no users yet, so this won’t be too disruptive.

sound-blaster-command
30 days: 3 (#4258)
90 days: 3 (#5862)
365 days: 3 (#7143)
Age: 86 days (added 2020, June 05)


uninstall script: {

executable: '/Applications/Creative/Uninstaller.app/Contents/MacOS/Uninstaller',
Copy link
Member

@vitorgalvao vitorgalvao Aug 31, 2020

Choose a reason for hiding this comment

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

@ran-dall For future reference, we don’t allow this as an uninstall because it’s not a script, it invokes the uninstaller GUI which holds up the terminal indefinitely.

We’d need #24377 to use this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants