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

Jump to bottom

fix: improving URLs on linuxbrew #1900

Merged
merged 6 commits into from Nov 16, 2020
Merged

fix: improving URLs on linuxbrew #1900

merged 6 commits into from Nov 16, 2020

Conversation

caarlos0
Copy link
Member

  • rename some template fields
  • changing the ifs to avoid empty if statements
  • adding depends_on :linux if only linux URLs

closes #1893
maybe helps with #1804

@caarlos0
fix: improving URLs on linuxbrew
0d9df8b 
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
@caarlos0
refactor: rename template fields
dfc7100 
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
@caarlos0 caarlos0 added the bug Something isn't working label Nov 13, 2020
@caarlos0 caarlos0 self-assigned this Nov 13, 2020
@vercel
Copy link

vercel bot commented Nov 13, 2020 •
edited

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/goreleaser/goreleaser/5ijx0bsqv
✅ Preview: https://goreleaser-git-brew-improvements.goreleaser.vercel.app

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 13, 2020
caarlos0
caarlos0 commented Nov 13, 2020
View reviewed changes
internal/pipe/brew/brew.go
@@ -310,21 +310,21 @@ func dataFor(ctx *context.Context, cfg config.Homebrew, cl client.Client, artifa
result.MacOS = down
} else if artifact.Goos == "linux" {
switch artifact.Goarch {
case "386", "amd64":
if result.Linux.DownloadURL != "" {
case "amd64":
Copy link
Member Author

Choose a reason for hiding this comment

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

we never include i386 builds, so this part of the switch was useless

This was referenced Nov 13, 2020
Closed
Closed
radeksimko
radeksimko approved these changes Nov 14, 2020
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

I wonder how soon are we going to need to make a similar change to account for darwin/arm64 (the Apple Silicon) 😄

internal/pipe/brew/testdata/test.rb.golden Outdated
@@ -4,24 +4,21 @@ class Test < Formula
homepage "https://google.com"
version "0.1.3"
bottle :unneeded

Copy link
Member

Choose a reason for hiding this comment

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

I think that extra space was actually useful from grouping perspective.

@caarlos0
Copy link
Member Author

Once go 1.16 is out I believe 😂

@vercel vercel bot temporarily deployed to Preview November 15, 2020 22:07 Inactive
@codecov
Copy link

codecov bot commented Nov 15, 2020 •
edited

Codecov Report

Merging #1900 (3100939) into master (40aa04f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1900   +/-   ##
=======================================
  Coverage   81.78%   81.78%           
=======================================
  Files          73       73           
  Lines        3827     3827           
=======================================
  Hits         3130     3130           
  Misses        573      573           
  Partials      124      124
Impacted Files Coverage Δ
internal/pipe/brew/brew.go 78.45% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40aa04f...3100939. Read the comment docs.

mavogel
mavogel approved these changes Nov 15, 2020
View reviewed changes
Copy link
Member

@mavogel mavogel left a comment

Choose a reason for hiding this comment

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

LGTM. Fixed the golden files to make the tests green and docs

@caarlos0
Revert "fix(tests): adapts brew test formulas and docs"
a3bb833 
This reverts commit 51dd8cf.
@caarlos0
fix: template
08ef9df 
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
@vercel vercel bot temporarily deployed to Preview November 16, 2020 01:25 Inactive
@caarlos0
Copy link
Member Author

LGTM. Fixed the golden files to make the tests green and docs

the tests were right to fail, bad sed on my side... reverted your commit...

@caarlos0
docs: fixed example
3100939 
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
@vercel vercel bot temporarily deployed to Preview November 16, 2020 01:29 Inactive
@caarlos0 caarlos0 merged commit b4f154d into master Nov 16, 2020
@caarlos0 caarlos0 deleted the brew-improvements branch November 16, 2020 12:16
@github-actions
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radeksimko radeksimko radeksimko approved these changes

@mgrachev mgrachev mgrachev approved these changes

@mavogel mavogel mavogel approved these changes

@crazy-max crazy-max Awaiting requested review from crazy-max

Assignees

@caarlos0 caarlos0

Labels
bug Something isn't working size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Releasing to a Linux-only Homebrew formula
4 participants
@caarlos0 @radeksimko @mgrachev @mavogel