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

Update cayley.rb #49841

Open
wants to merge 18 commits into
base: master
from
Open

Update cayley.rb #49841

wants to merge 18 commits into from

Conversation

@iddan
Copy link

iddan commented Feb 6, 2020

  • Update installation to bundle UI correctly

  • Update homepage

  • Update desc

  • Reduced duplicities in installation

  • Updated used configuration file

  • Removed unused dependencies

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

  • Is your test running fine brew test <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>)?


 - Use GitHub release prebuilt binary (fixes UI not showing for brew installations)
 - Update homepage
 - Update desc
@iddan iddan mentioned this pull request Feb 6, 2020
Copy link
Member

SMillerDev left a comment

This goes against almost all of the rules for homebrew-core. I marked the most important ones.

Formula/cayley.rb Outdated Show resolved Hide resolved
Formula/cayley.rb Outdated Show resolved Hide resolved
Formula/cayley.rb Outdated Show resolved Hide resolved
iddan added 12 commits Feb 7, 2020
@iddan iddan marked this pull request as ready for review Feb 7, 2020
@iddan

This comment has been minimized.

Copy link
Author

iddan commented Feb 7, 2020

@SMillerDev may you please re-review? Thank you for the patience to highlight which rules were broken.

@bayandin

This comment has been minimized.

Copy link
Member

bayandin commented Feb 8, 2020

There're some linter finds, that should be addressed:

Error Message
failed: brew audit cayley --online
Stacktrace
        Error: 3 problems in 1 formula detected
cayley:
  * Stable: version 0.7.7 is redundant with version scanned from URL
  * stable: sha256 changed without the version also changing; please create an issue upstream to rule out malicious circumstances and to find out why the file changed.
  * C: 41: col 7: Do not use `go get`. Please ask upstream to implement Go vendoring
Error Message
failed: brew style cayley
Stacktrace
        == /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/cayley.rb ==
C: 18: 24: Freeze mutable objects assigned to constants.
C: 19: 14: Freeze mutable objects assigned to constants.
C: 35:  1: Trailing whitespace detected.
C: 57:  7: Don't use parentheses around a variable.

1 file inspected, 4 offenses detected
@iddan

This comment has been minimized.

Copy link
Author

iddan commented Feb 8, 2020

Thank you for posting. I'll try to fix all the issues.

`* stable: sha256 changed without the version also changing; please create an issue upstream to rule out malicious circumstances and to find out why the file changed.

The sha was changed because the binary includes the UI now. While it didn't before.`

iddan added 5 commits Feb 8, 2020
@iddan

This comment has been minimized.

Copy link
Author

iddan commented Feb 16, 2020

I have a problem with making the build work. Can someone help me out?

depends_on "go" => :build
depends_on "mercurial" => :build
depends_on "gobuffalo/tap/packr" => :build

This comment has been minimized.

Copy link
@equal-l2

equal-l2 Feb 16, 2020

Contributor

You shouldn't use personal tap in a core formula.

url "https://github.com/cayleygraph/cayley.git",
:tag => "v0.7.7",
:revision => "dcf764fef381f19ee49fad186b4e00024709f148"
sha256 "37ef9043cb20aeffa93d57f360fdee3e0c9d1b2fa8d404a00f3bff1b58937206"

This comment has been minimized.

Copy link
@equal-l2

equal-l2 Feb 16, 2020

Contributor

If you want to change sha256 without changing the version, you may want to use revision.
See https://docs.brew.sh/Formula-Cookbook#formulae-revisions.

This comment has been minimized.

Copy link
@SMillerDev

SMillerDev Feb 16, 2020

Member

In addition to that, why did the hash change? Is upstream aware of this and was it intentional?

This comment has been minimized.

Copy link
@equal-l2

equal-l2 Feb 16, 2020

Contributor

Sorry, I misunderstood the situation.
The sha256 is not changed, but added by mistake.
The source is downloaded via git, so the formula doesn't need the sha256.


@iddan Please remove the sha256, since this is not needed.

This comment has been minimized.

Copy link
@iddan

iddan Feb 16, 2020

Author

Okay, will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.