Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

glyr 1.0.8 (new formula) and optional dependency in abcde #44364

Closed
wants to merge 2 commits into from

Conversation

rwstauner
Copy link
Contributor

I'm not sure what the guidelines/preferences are for optional dependencies, but if your abcde config file specifies getalbumart in ACTIONS abcde will refuse to run if it cannot find glyrc.

class Glyr < Formula
desc "Music related metadata searchengine with command-line interface and C API"
homepage "https://github.com/sahib/glyr"

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this empty line.

@rwstauner
Copy link
Contributor Author

If this library is too obscure that's fine. I hadn't heard of it myself until trying to use this new abcde feature and got an error that this thing was missing.
I considered just making a tap for it, but thought i'd start with this since abcde does expect it under certain configurations.

end

test do
(out = testpath/"cover.txt").write("x")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need to create the file before running the command below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I suppose not. It's just an old habit of initializing it so you can be certain it changed during the test.
Is testpath a new directory every time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and rebased. Thanks!

@rwstauner
Copy link
Contributor Author

This is my first new formula so I appreciate you taking the time to comment.

test do
out = testpath/"cover.txt"
system "#{bin}/glyrc", "cover", "-D", "--artist", "Beatles", "--album", "Please Please Me", "-w", out
assert File.read(out).match(%r{^https?://})
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use assert_match %r{^https?://}, out.read here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sweet, thanks.

The getalbumart function keeps the program from running without glyr.
@dunn
Copy link
Contributor

dunn commented Oct 3, 2015

Notable enough, and important to have dependencies of our existing formula. Merged in b7b470a, thanks!

@dunn dunn closed this in b7b470a Oct 3, 2015
@rwstauner
Copy link
Contributor Author

Thanks!

@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants