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

chruby-fish 0.8.0 #1266

Closed
wants to merge 1 commit into from
Closed

chruby-fish 0.8.0 #1266

wants to merge 1 commit into from

Conversation

JeanMertz
Copy link
Contributor

@JeanMertz JeanMertz commented May 18, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Have you built your formula locally prior to submission with brew install <formula> (where <formula> is the name of the formula you're submitting)?
  • Does your submission pass brew audit --strict --online <formula> (after doing brew install <formula>)?

The last one is unchecked due to:

$ brew audit --strict --online chruby-fish
chruby-fish:
  * A `test do` test block should be added
  * GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)
Error: 2 problems in 1 formula

I guess there is a new policy about package popularity? Should the package be removed from Homebrew? (those numbers are currently at 14, 7, 45).

@MikeMcQuaid
Copy link
Member

  • A test do test block should be added

Would be good to do this if you could!

  • GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)

We only use this as a judge for new formulae.

@JeanMertz
Copy link
Contributor Author

JeanMertz commented May 18, 2016

  • A test do test block should be added

Would be good to do this if you could!

I was already looking into this, taking some inspiration from existing tests.

For any substantial test to work though, I have to work with the fish shell/binary, something I suspect won't be available on the build servers.

Any thoughts on how to tackle this?

@MikeMcQuaid
Copy link
Member

Feels like this should probably depends_on "fish" => :recommended anyway which would mean you have it available at test time.

@JeanMertz
Copy link
Contributor Author

Thanks 👍 fixed.


test do
output = `fish -ic '. #{share}/chruby/chruby.fish; chruby --version'`
assert output.include?("chruby-fish")
Copy link
Member

Choose a reason for hiding this comment

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

Use assert_match instead of assert ...include?

@JeanMertz JeanMertz force-pushed the patch-1 branch 2 times, most recently from 9eecd80 to ca97baf Compare May 18, 2016 18:00
@MikeMcQuaid
Copy link
Member

Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

@JeanMertz JeanMertz deleted the patch-1 branch May 18, 2016 23:18
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 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.

None yet

2 participants