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

search: use single HTTP call for tap searches. #2540

Merged
merged 1 commit into from Apr 24, 2017

Conversation

Projects
None yet
3 participants
@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Apr 24, 2017

Use GitHub's code search API to search using the filename based on the search query. This means we only need a single HTTP call and no more multithreading madness. This also means we're able to search everything in the Homebrew and Caskroom organisation by default without having to maintain a list of things to search (and not) in here.

CC @reitermarkus as we talked about this in #2254.

@MikeMcQuaid MikeMcQuaid referenced this pull request Apr 24, 2017

Closed

[WIP] `brew search` freezes on some GitHub API responses #2466

10 of 11 tasks complete
search: use single HTTP call for tap searches.
Use GitHub's code search API to search using the filename based on the
search query. This means we only need a single HTTP call and no more
multithreading madness. This also means we're able to search everything
in the Homebrew and Caskroom organisation by default without having to
maintain a list of things to search (and not) in here.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:search-taps-code-search branch from 7815e68 to b3c69ab Apr 24, 2017

@reitermarkus

This comment has been minimized.

Copy link
Member

reitermarkus commented Apr 24, 2017

@MikeMcQuaid, moved some of the code into the GitHub module.

open(URI.parse("#{API_URL}/repos/#{user}/#{repo}")) { |j| j }
end

def search_code(filename: nil, filenames: nil, user: nil, users: nil, &block)

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 24, 2017

Member

@reitermarkus Can you revert this for now so it can be shipped as it was and refactor into this form if/when you/we need to use it elsewhere and with tests? Thanks.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:search-taps-code-search branch from 6822968 to b3c69ab Apr 24, 2017

@MikeMcQuaid MikeMcQuaid merged commit b54f5c4 into Homebrew:master Apr 24, 2017

3 checks passed

codecov/patch 70.83% of diff hit (target 64.43%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +6.4% compared to 581e3b7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:search-taps-code-search branch Apr 24, 2017

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 24, 2017

@MikeMcQuaid, moved some of the code into the GitHub module.

@reitermarkus Let's leave this as-is until we have either other callsites and/or tests.

@joshuata

This comment has been minimized.

Copy link

joshuata commented Apr 24, 2017

This looks like it might be causing issues with Jenkins. We just had a job fail on Homebrew/homebrew-core#11593 with the error:

...
7 gems installed
Error: undefined method `search_tap' for Homebrew:Module
...

(edited to correct grammar)

@joshuata joshuata referenced this pull request Apr 25, 2017

Closed

Error: undefined method `search_tap' for Homebrew:Module #2544

4 of 4 tasks complete

MikeMcQuaid added a commit to MikeMcQuaid/brew that referenced this pull request Apr 25, 2017

audit: fix use of search_tap method.
This was removed in Homebrew#2540 but this call site was note updated to use
the `search_taps` method instead.

MikeMcQuaid added a commit to MikeMcQuaid/brew that referenced this pull request Apr 25, 2017

audit: fix use of search_tap method.
This was removed in Homebrew#2540 but this call site was note updated to use
the `search_taps` method instead.

mansimarkaur added a commit to mansimarkaur/brew that referenced this pull request Aug 5, 2017

audit: fix use of search_tap method.
This was removed in Homebrew#2540 but this call site was note updated to use
the `search_taps` method instead.

@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.