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

audit: stop demanding a HTTP HEAD mirror for curl #3126

Merged
merged 1 commit into from Sep 12, 2017

Conversation

Projects
None yet
2 participants
@DomT4
Contributor

DomT4 commented Sep 5, 2017

  • 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 successfully run brew tests with your changes locally?

Not sure if this is how you want to handle it but having a HEAD mirror for curl is just silliness.

Ref: Homebrew/homebrew-core@e36b958

audit: stop demanding a HTTP HEAD mirror for curl
Not sure if this is how you want to handle it but having a HEAD mirror
for `curl` is just silliness.

Ref: Homebrew/homebrew-core@e36b958
@@ -1237,7 +1237,7 @@ def audit_download_strategy
def audit_urls
urls = [url] + mirrors
if name == "curl" && !urls.find { |u| u.start_with?("http://") }
if name == "curl" && !urls.find { |u| u.start_with?("http://") } && url != Formula["curl"].head.url

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Sep 5, 2017

Member

Add the url != Formula["curl"].head.url into the urls.find and maybe split it into a separate variable like http_non_stable_urls and also filter out devel and do next if url == Formula["curl"].head.url.

@MikeMcQuaid

MikeMcQuaid Sep 5, 2017

Member

Add the url != Formula["curl"].head.url into the urls.find and maybe split it into a separate variable like http_non_stable_urls and also filter out devel and do next if url == Formula["curl"].head.url.

This comment has been minimized.

@DomT4

DomT4 Sep 6, 2017

Contributor

Sure. Happy to take a run at it that way. Thought I'd open with the simplest possible fix & see what the feedback was, since I have something of a habit of overcomplicating solutions right out of the gate 😄.

@DomT4

DomT4 Sep 6, 2017

Contributor

Sure. Happy to take a run at it that way. Thought I'd open with the simplest possible fix & see what the feedback was, since I have something of a habit of overcomplicating solutions right out of the gate 😄.

@MikeMcQuaid MikeMcQuaid merged commit ef60688 into Homebrew:master Sep 12, 2017

1 of 3 checks passed

codecov/patch 0% of diff hit (target 53.94%)
Details
codecov/project 53.93% (-0.02%) compared to e777010
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 12, 2017

Member

Touching this code in #3151 so will rebase mine on this. Thanks @DomT4!

Member

MikeMcQuaid commented Sep 12, 2017

Touching this code in #3151 so will rebase mine on this. Thanks @DomT4!

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Sep 12, 2017

Contributor

Thanks Mike. Sorry I hadn't touched this one again yet; this last week has been... not glacially paced 😅.

Contributor

DomT4 commented Sep 12, 2017

Thanks Mike. Sorry I hadn't touched this one again yet; this last week has been... not glacially paced 😅.

@DomT4 DomT4 deleted the DomT4:curl_audit branch Sep 12, 2017

@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.