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

ttyd 1.1.0 (new formula) #5310

Closed
wants to merge 1 commit into from
Closed

ttyd 1.1.0 (new formula) #5310

wants to merge 1 commit into from

Conversation

tsl0922
Copy link
Contributor

@tsl0922 tsl0922 commented Sep 27, 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 --new-formula <formula> (after doing brew install <formula>)?

@zmwangx zmwangx added the new formula PR adds a new formula to Homebrew/homebrew-core label Sep 28, 2016
Copy link
Contributor

@zmwangx zmwangx left a comment

Choose a reason for hiding this comment

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

Overall 👍 However,

$ curl -sS https://api.github.com/repos/tsl0922/ttyd | grep created_at
  "created_at": "2016-09-13T01:43:54Z",

the repo is only two weeks old and not massively popular, so I'd rather stick to our 30 day rule and sit on it.

end

test do
system "#{bin}/ttyd", "--version"
Copy link
Contributor

Choose a reason for hiding this comment

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

assert_match version.to_s, shell_output("#{bin}/ttyd --version")

head "https://github.com/tsl0922/ttyd.git"

depends_on "cmake" => :build
# xxd tool is from vim
Copy link
Contributor

@zmwangx zmwangx Sep 28, 2016

Choose a reason for hiding this comment

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

Not necessary, remove this comment.

However, isn't /usr/bin/xxd good enough?

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 should use which -a xxd to find the location of xxd 😄 , will remove dependency for vim.

def install
cmake_args = std_cmake_args + ["-DOPENSSL_ROOT_DIR=#{Formula["openssl"].opt_prefix}"]
system "cmake", ".", *cmake_args
system "make"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary, remove this line.

@tsl0922
Copy link
Contributor Author

tsl0922 commented Sep 28, 2016

@zmwangx PTAL

sha256 "6fae4e932280f0ca430ea834f88282673351a321535acbe3725e995e83857138"
head "https://github.com/tsl0922/ttyd.git"

depends_on "cmake" => :build
Copy link
Contributor

Choose a reason for hiding this comment

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

According to CI failure we need to

depends_on "pkg-config" => :build

I didn't catch that locally because I have it installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks.

@zmwangx
Copy link
Contributor

zmwangx commented Sep 30, 2016

Now the only problem is repo too new. I suggest we sit on this for two weeks, by which time the audit failure will resolve itself. It'll also give the project time to mature a bit more if necessary.

@tsl0922
Copy link
Contributor Author

tsl0922 commented Sep 30, 2016

Thanks @zmwangx. I'm OK with the 30 day rule.

@tsl0922 tsl0922 changed the title ttyd 1.0.0 (new formula) ttyd 1.1.0 (new formula) Oct 13, 2016
@tsl0922
Copy link
Contributor Author

tsl0922 commented Oct 13, 2016

@zmwangx I'v updated the formula to 1.1.0 release with some new features and minor bug fixes.

@zmwangx
Copy link
Contributor

zmwangx commented Oct 13, 2016

👍

@MikeMcQuaid
Copy link
Member

Pulled it as it's getting releases and is more popular now. Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

@tsl0922 tsl0922 deleted the ttyd branch October 13, 2016 13:19
@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