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

marathon-swift 1.0.1 (new formula) #17266

Closed
wants to merge 4 commits into from
Closed

marathon-swift 1.0.1 (new formula) #17266

wants to merge 4 commits into from

Conversation

orta
Copy link
Contributor

@orta orta commented Aug 26, 2017

  • 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>)?

Adds Marathon: https://github.com/JohnSundell/Marathon

@orta
Copy link
Contributor Author

orta commented Aug 26, 2017

Ah dang, I thought that linter error was just me setting up my local dev env wrong. Interesting, will discuss with the maintainers about naming.

@orta orta changed the title marathon 1.0.1 (new formula) marathon-swift 1.0.1 (new formula) Aug 26, 2017
@orta orta force-pushed the patch-1 branch 6 times, most recently from 758a8b8 to 14e82e8 Compare August 26, 2017 21:04
@orta
Copy link
Contributor Author

orta commented Aug 26, 2017

Awesome, we're green 👍

@ilovezfs ilovezfs added the new formula PR adds a new formula to Homebrew/homebrew-core label Aug 27, 2017
depends_on :xcode => ["8.3", :build]

def install
ENV["SDKROOT"] = MacOS.sdk_path
Copy link
Contributor

Choose a reason for hiding this comment

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

This is set automatically.

@@ -0,0 +1,27 @@
class MarathonSwift < Formula
desc "Makes it easy to write, run and manage your Swift scripts 🏃"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave out the emoji

system "swift", "build", "-c", "release", "-Xswiftc", "-static-stdlib"
end

bin.install ".build/release/marathon"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please request a make install upstream.

end

test do
system "#{bin}/marathon", "help"
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be a test that exercises and verifies the functionality of the software once installed.

@orta
Copy link
Contributor Author

orta commented Aug 27, 2017

@ilovezfs - I've shipped ~10 force-commits to address your feedback, but do you know if there's anything unique about the Sierra and using the install command? I've tried building out a make command for installing the app that calls:

PREFIX?=/usr/local
INSTALL_NAME = marathon

# [...]

install_bin:
	mkdir -p $(PREFIX)/bin
	mv .build/Release/Marathon .build/Release/$(INSTALL_NAME)
	/usr/bin/install -s -m 0755 .build/Release/$(INSTALL_NAME) $(PREFIX)/bin

But it only fails on Sierra, and when I tried using cp it was the same story. This is my first time making cask / using Makefiles for system setup, so am I missing something simple?

@orta
Copy link
Contributor Author

orta commented Aug 29, 2017

@ilovezfs this is now green 👍

@orta
Copy link
Contributor Author

orta commented Aug 30, 2017

Polite Poke

@ilovezfs
Copy link
Contributor

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

@ilovezfs ilovezfs closed this in 3a922fc Aug 31, 2017
@orta
Copy link
Contributor Author

orta commented Aug 31, 2017

No - you rock - thanks!

@JohnSundell
Copy link

@orta @ilovezfs you both rock 🎸 Thanks a lot for this!

@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
new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants