Cassandra-reaper 1.3.0 (new formula)

[kotaroyamazaki]

• Have you followed the guidelines for contributing?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• [] Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

---

Since this formula produce cross-platform binaries (Java), it does not use brew install --build-from-source <formula>.

The inline patch is temporary to update the 'cassandra-reaper' script for brew.
Since PR thelastpickle/cassandra-reaper#533 is merged, it will be obsolete once next version is released.

[BrewTestBot]

• Formulae should not require patches to build. Patches should be submitted and accepted upstream first.

[muru]

@DomT4 (cc: @BrewTestBot - not sure who that goes to) Is the presence of a patch a hard blocker against the inclusion of a new formula? The patch has already been accepted upstream prior to this PR (in fact the link to the patch is from the upstream commit). But it's not clear when the next release will be and the fix seems to be too minor to warrant a new point release by itself.

[SMillerDev]

BrewTestBot is a... Bot. It'll just ignore you 😄. As for the patch, I'd say it's less problematic because it's already accepted upstream.

There is a lot of mention of "for homebrew" in the formula though, which I think is kind of implied in a homebrew formula. (For example in the description) could you remove those descriptions?

[kotaroyamazaki]

Thanks for the feedback, I have re-pushed with those changes.

[SMillerDev]

It seems these are jar files, those are usually packed in libexec for homebrew.

[kotaroyamazaki]

Yes, these are jar files.
I think it is no problem because those in the other package of casssandra-reaper are also packed in share.

[DomT4]

BrewTestBot is a... Bot. It'll just ignore you 😄

To be fair I do a fair bit of talking to the bot 😆.

I think we can live without the patch though because, well, it doesn't actually fix the whole Homebrew problem because it assumes HOMEBREW_PREFIX == /usr/local, and that's not something we mandate. It looks like we could write an env wrapper around cassandra-reaper to set CLASS_PATH and CONFIG_PATH ourselves? In which case that would be the avenue to go down.

[muru]

Would a wrapper like the one in Formula/hornetq.rb be acceptable?

[DomT4]

This style of syntax is slightly cleaner & should work fine:

env = {
  :PATH => "#{libexec}/vendor/bin:$PATH",
  :PYTHONPATH => ENV["PYTHONPATH"],
}
bin.env_script_all_files(libexec/"bin", env)

If there are multiple files in that directory and you only need to script one you can use write_env_script instead of env_script_all_files. There's a good example of that in corsixth.

[kotaroyamazaki]

@DomT4
Thanks for the feedback.
But I think it is difficult, because the script 'cassandra-reaper' inserts CONFIG_PATH and CLASS_PATH directly.
So even if we set environment path, it seems to be rewritten when cassandra-reaper is executed.
Or may I misunderstand something about how to use write_env_script?

[kotaroyamazaki]

@DomT4
I'm sorry, I misunderstand your mention a little and I've tried it.
About CLASS_PATH, we can set env using write_env_script.

About CONFIG_PATH, there is a problem.
cassandra-reaper script about CLASS_PATH is written followed .

if [ $# -eq 0 ]; then
    CONFIG_PATH="/etc/cassandra-reaper/cassandra-reaper.yaml"
  else
    CONFIG_PATH="$@"

Because of such condition, if not have argument it set automatically.
We can modify the condition and PR, but it means to need patch...
Do you have any idea?

[DomT4]

Do you have any idea?

Does it work if we inreplace <path/to/cassandra-reaper>, "etc", etc?

That would replace the /etc with HOMEBREW_PREFIX/etc, usually /usr/local/etc.

[kotaroyamazaki]

@DomT4
Thanks for great advice.
I've fixed and made it work without a patch.

[kotaroyamazaki]

@DomT4
PTAL at the last commit.

[DomT4]

Combine this line & the line above into:

pkgshare.install "server/target" => "cassandra-reaper"

[kotaroyamazaki]

With such a description, the path of the jar file will change and it will not work.

[kotaroyamazaki]

@DomT4
Thank you for the great review. I've re-pushed with those changes.
Please check this.

[kotaroyamazaki]

@DomT4
PTAL at the last commit.

[kotaroyamazaki]

Thanks @muru.
I've update for a new release.
And confirmed that it passes brew test and brew audit --strict.
If there is no problem, please merge.

[kotaroyamazaki]

@SMillerDev
PTAL at the last commit.
It is seemed to be solved all problems.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[kotaroyamazaki]

Is there no reviewer for this ... ?

[SMillerDev]

@BrewTestBot test this please!

[SMillerDev]

If it produces java files you need to define a Java version it depends on.

[javian]

They seem to be using 1.8 on Travis

jdk:
- oraclejdk8

so either add depends_on :java => "1.8" or depends_on :java => "1.8+"

[kotaroyamazaki]

@SMillerDev @javian
Thank you for the great review. I've re-pushed with those changes.
Please check this.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[fxcoudert]

Thanks @kotaroyamazaki for your contribution to Homebrew!