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

initial ctop formula #10846

Closed
wants to merge 6 commits into from
Closed

initial ctop formula #10846

wants to merge 6 commits into from

Conversation

avelino
Copy link
Contributor

@avelino avelino commented Mar 10, 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>)?

bcicen/ctop#5

@bfontaine
Copy link
Contributor

For the future pull-requests please follow our guidelines for contributing and use the commit format ctop 0.4.1 (new formula). Thanks!

Formula/ctop.rb Outdated
class Ctop < Formula
desc "Top-like interface for container metrics"
homepage "https://bcicen.github.io/ctop/"
url "https://github.com/bcicen/ctop/releases/download/v0.4.1/ctop-0.4.1-darwin-amd64"
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to build from source here.

Formula/ctop.rb Outdated

def install
mv "ctop-0.4.1-darwin-amd64", "ctop"
bin.install "ctop"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don’t use mv + bin.install. See my comment on your other PR: #10847 (comment)

@woodruffw woodruffw added the new formula PR adds a new formula to Homebrew/homebrew-core label Mar 10, 2017
Formula/ctop.rb Outdated
(buildpath/"src/github.com/bcicen").mkpath
ln_s buildpath, buildpath/"src/github.com/bcicen/ctop"
system "go", "build", "-o", "ctop"
bin.install "ctop"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can merge this line and the one above by using system "go", "build", "-o", bin/"ctop".

@avelino
Copy link
Contributor Author

avelino commented Mar 11, 2017

Awaiting project solve that problem to send update for homebrew: bcicen/ctop#33

PR on ctop to fixed problem: bcicen/ctop#34 (awaiting approval)

@fxcoudert
Copy link
Member

  * `bottle modifier` (line 8) should be put before `depends_on` (line 7)
  * C: 7: col 14: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Formula/ctop.rb Outdated
class Ctop < Formula
desc "Top-like interface for container metrics"
homepage "https://bcicen.github.io/ctop/"
url "https://github.com/bcicen/ctop/archive/v0.4.1-deps.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

This -deps confuses the version parser; please add the version below this line using version "0.4.1".

Error: 1 problem in 1 formula
ctop:
  * Stable: version (deps) is set to a string without a digit

@avelino
Copy link
Contributor Author

avelino commented Mar 13, 2017

@bcicen pls

@bcicen
Copy link
Contributor

bcicen commented Mar 13, 2017

@avelino - as @bfontaine's comment suggests, all you need to do is insert

version "0.4.1"

below line 4 in the current formula

@avelino
Copy link
Contributor Author

avelino commented Mar 13, 2017

@bcicen
Formula:

class Ctop < Formula
  desc "Top-like interface for container metrics"
  homepage "https://bcicen.github.io/ctop/"
  version "0.4.1"
  url "https://github.com/bcicen/ctop/archive/v0.4.1-deps.tar.gz"
  sha256 "bc10b774dad0bc7ef0be41bcfb36c774fc28dafa789b2f43f1ecdb5b75390867"

  depends_on "go" => :build
  depends_on "glide" => :build

  def install
    ENV["GOPATH"] = buildpath
    ENV["GLIDE_HOME"] = HOMEBREW_CACHE/"glide_home/#{name}"
    dir = buildpath/"src/github.com/bcicen/ctop"
    dir.install Dir["*"]
    cd dir do
      system "glide", "install"
      system "go", "build", "-o", bin/"ctop"
    end
  end

  test do
    system "#{bin}/ctop", "-v"
  end
end

Error:

$ brew install --build-from-source Formula/ctop.rb
==> Downloading https://github.com/bcicen/ctop/archive/v0.4.1-deps.tar.gz
Already downloaded: /Users/avelino/Library/Caches/Homebrew/ctop-0.4.1.tar.gz
==> glide install
Last 15 lines from /Users/avelino/Library/Logs/Homebrew/ctop/01.glide:
2017-03-13 11:11:42 -0300

glide
install

[ERROR] Failed to find glide.yaml file in directory tree: Cannot resolve parent of /

Do not report this issue to Homebrew/brew or Homebrew/core!

These open issues may also help:
initial ctop formula https://github.com/Homebrew/homebrew-core/pull/10846

@avelino
Copy link
Contributor Author

avelino commented Mar 13, 2017

@bcicen Will generate new release?

@bcicen
Copy link
Contributor

bcicen commented Mar 13, 2017

@avelino you need to clear your local cache before testing the brew install again to fetch the correct version:

rm -rvf /Users/avelino/Library/Caches/Homebrew/ctop*

@bcicen
Copy link
Contributor

bcicen commented Mar 14, 2017

@avelino version needs to be on line 5, below url. Test error:
* `url` (line 5) should be put before `version` (line 4)

Avelino added 2 commits March 13, 2017 23:32
```
$ brew install --build-from-source Formula/ctop.rb
==> Downloading https://github.com/bcicen/ctop/archive/v0.4.1-deps.tar.gz
Already downloaded: /Users/avelino/Library/Caches/Homebrew/ctop-0.4.1.tar.gz
==> glide install
==> go build -o /usr/local/Cellar/ctop/0.4.1/bin/ctop
🍺  /usr/local/Cellar/ctop/0.4.1: 3 files, 7.4MB, built in 9 seconds
```
@fxcoudert
Copy link
Member

Thanks @avelino for your contribution to Homebrew!

@fxcoudert fxcoudert closed this in a6a161b Mar 15, 2017
@avelino avelino deleted the ctop branch March 15, 2017 16:37
@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

5 participants