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

New formula: Zopfli v1.0.0 #19435

Closed
wants to merge 1 commit into from
Closed

New formula: Zopfli v1.0.0 #19435

wants to merge 1 commit into from

Conversation

mathiasbynens
Copy link
Contributor

@mistydemeo
Copy link
Member

You don't need to close and submit a new pull request if you want to make a change to it. You can just force-push to the same branch on Github and it'll update automatically.

@mathiasbynens
Copy link
Contributor Author

@mistydemeo I know, but the other pull request (#18150) isn’t mine, so that wouldn’t work. But thank you all the same :)

@mistydemeo
Copy link
Member

Sorry, I somehow missed that the other pull request from you was in homebrew/headonly.


class Zopfli < Formula
homepage 'https://code.google.com/p/zopfli/'
version '1.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

No need for an explicit version, this is detected from the URL.

@mathiasbynens
Copy link
Contributor Author

Thanks for the quick review, @mistydemeo! I’ve amended my commit.

@mathiasbynens
Copy link
Contributor Author

@adamv

Use the full path to binaries in test methods: system "#{bin}/zopfli"

Done. Force-pushed the amended commit.

end

test do
system '#{bin}/zopfli'
Copy link
Member

Choose a reason for hiding this comment

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

Single quotes don't get string interpolation - need to use double quotes 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.

Thanks 😊 Amended.

Closes #18150 and #19435.

Signed-off-by: Mathias Bynens <mathias@qiwi.be>
@mistydemeo
Copy link
Member

Great! If you could report that to the author that would be great. It would be best if it did something like this:

diff --git a/makefile b/makefile
index 7e3272c..4d5488c 100644
--- a/makefile
+++ b/makefile
@@ -1,5 +1,7 @@
+CC := gcc
+
 make:
-   gcc src/zopfli/*.c -O2 -W -Wall -Wextra -ansi -pedantic -lm -o zopfli
+   $(CC) src/zopfli/*.c -O2 -W -Wall -Wextra -ansi -pedantic -lm -o zopfli

 debug:
-   gcc src/zopfli/*.c -g3 -lm -o zopfli
+   $(CC) src/zopfli/*.c -g3 -lm -o zopfli

So that we could specify our CC on the commandline.

@mathiasbynens
Copy link
Contributor Author

Will do. Thank you so much for your patience and the many helpful comments!

Update: Done here.

mistydemeo pushed a commit that referenced this pull request May 1, 2013
Closes #18150 and #19435.

Signed-off-by: Mathias Bynens <mathias@qiwi.be>
Signed-off-by: Misty De Meo <mistydemeo@gmail.com>
@mistydemeo
Copy link
Member

Pulled, thanks!

@alrra
Copy link

alrra commented May 1, 2013

woo hoo! \o/

@adamv adamv closed this May 1, 2013
@mathiasbynens
Copy link
Contributor Author

@mistydemeo Zopfli’s makefile has been updated: https://code.google.com/p/zopfli/source/browse/makefile There are now CC, CXX, CFLAGS, CXXFLAGS settings. Will this do?

@mistydemeo
Copy link
Member

Looks good! We can make use of those when the next release is out. Thanks for taking this up with the author, I appreciate it.

@mathiasbynens
Copy link
Contributor Author

Okay, I’ll ask the author to bump the version number and will update the Homebrew formula as soon as that happens. Thanks so much for the guidance, @mistydemeo!

handyman5 pushed a commit to handyman5/homebrew that referenced this pull request Oct 7, 2013
Closes Homebrew#18150 and Homebrew#19435.

Signed-off-by: Mathias Bynens <mathias@qiwi.be>
Signed-off-by: Misty De Meo <mistydemeo@gmail.com>
@Homebrew Homebrew locked and limited conversation to collaborators Feb 16, 2016
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

4 participants