Skip to content
This repository has been archived by the owner on Mar 12, 2019. It is now read-only.

pull: Fix fetching of x86_64_linux bottles #17

Closed
wants to merge 1 commit into from
Closed

pull: Fix fetching of x86_64_linux bottles #17

wants to merge 1 commit into from

Conversation

rwhogg
Copy link
Contributor

@rwhogg rwhogg commented May 4, 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 change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

If the tap you're pulling from includes "Linuxbrew" in its name and the formula has an x86_64_linux bottle, use that one. Otherwise, fall back to Homebrew's default behaviour.

Previously, we were verifying by downloading the first bottle in the list, but since that's usually a Mac bottle, the download will fail.

If the tap you're pulling from includes "Linuxbrew" in its name and
the formula has an x86_64_linux bottle, use that one. Otherwise,
fall back to Homebrew's default behaviour.
@rwhogg rwhogg self-assigned this May 4, 2016
@rwhogg rwhogg added the bug label May 4, 2016
@@ -516,7 +516,11 @@ def verify_bintray_published(formulae_names)
opoo "Cannot publish bottle: Failed reading info for formula #{f.full_name}"
next
end
bottle_info = jinfo.bottle_info(jinfo.bottle_tags.first)
if f.tap.remote.include?("Linuxbrew") && jinfo.bottle_tags.include?("x86_64_linux")
Copy link
Member

Choose a reason for hiding this comment

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

This incantation of f.tap.remote.include?("Linuxbrew") has come up a few times now. Would you like to factor out a tap.linux? function? It can be done in its own PR either before or after this PR as you see fit.

Copy link
Contributor Author

@rwhogg rwhogg May 4, 2016

Choose a reason for hiding this comment

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

Agreed that it'd be nice. Will spin off its own issue for it though, since otherwise I'd be changing a lot more stuff on this PR than I'd like to.

EDIT: spun off as #18

@rwhogg rwhogg closed this in 721d2cf May 4, 2016
@sjackman
Copy link
Member

sjackman commented May 4, 2016

Thanks for the fix!

@rwhogg rwhogg deleted the fix-pull branch September 22, 2016 22:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants