search: remove remote searching #15209
Conversation
Bo98
force-pushed
the
search-no-remote
branch
3 times, most recently
from
df23f10
to
c7b893d
Compare
There was a problem hiding this comment.
I think this make sense in the short-term because search seems to broken right now anyway. This should fix #15208.
Any of the other methods of adding the other cask taps to search can be handled in another PR. Either we can use #15107 for that or we can create a new issue to remind ourselves to follow up on that.
Library/Homebrew/brew.sh
Outdated
| @@ -810,6 +810,7 @@ fi | |||
| AUTO_UPDATE_COMMANDS=( | |||
| install | |||
| outdated | |||
| search | |||
There was a problem hiding this comment.
Why is this necessary now?
There was a problem hiding this comment.
Idea was this is the replacement for remote searching. The code we're removing always worked based on the latest remote state of the taps so doing an auto-update would keep similar behaviour.
There was a problem hiding this comment.
I'm conflicted on this. I see the benefit of this always being up-to-date. On the flip side, I've heard a bunch of people complain that "brew search is too slow" and I fear this may make it worse.
I'm tempted to say we merge without this and then be willing to add it in if we get any complaints or confusion over inconsistencies?
There was a problem hiding this comment.
Speed shouldn't really be any slower than brew outdated after this change, unless the slowness is in DidYouMean or something. Most of the slowness was likely the GitHub code search.
So not really convinced about that as a reason, but I am happy to drop it anyway.
There was a problem hiding this comment.
Great work here so far @Bo98. One tiny tweak and think this is good to go.
Library/Homebrew/brew.sh
Outdated
| @@ -810,6 +810,7 @@ fi | |||
| AUTO_UPDATE_COMMANDS=( | |||
| install | |||
| outdated | |||
| search | |||
There was a problem hiding this comment.
I'm conflicted on this. I see the benefit of this always being up-to-date. On the flip side, I've heard a bunch of people complain that "brew search is too slow" and I fear this may make it worse.
I'm tempted to say we merge without this and then be willing to add it in if we get any complaints or confusion over inconsistencies?
| @@ -42,51 +42,6 @@ def search_descriptions(string_or_regex, args, search_type: :desc) | |||
| end | |||
| end | |||
|
|
|||
| def search_taps(query, silent: false) | |||
I agree this is the best fit.
IMO this is yet another good reason to combine these taps. I still think we should be combining cask-fonts too, at least for those that are clearly used through analytics CC @Homebrew/cask. |
It's possible we could have formulae.brew.sh support for cask-fonts, I wouldn't say that's completely out of the question. Focus on the others first though. |
I don't think it's worth it for a single tap. I think there's a host of issues with us having multiple official cask taps in the Homebrew organisation. |
|
We're talking about 2000 packages here though rather than a couple, which would increase the size of homebrew-cask by 50%. There's maintainability questions that would be best to discuss with the people who maintain it on a day-to-day basis. If there are benefits, then it would absolutely be worth it - formulae.brew.sh integration isn't really that tough of a task. Though I'm not the person to ask about benefits of that tap since I don't maintain it. |
I think using analytics can slim this down a bit but, post-sharding: I think this is still worthwhile.
I think maintainability is important but: so is user/contributor experience and the current level of discoverability makes both of those harder.
Yeh, probably but but: we've got a host of other API-related issues affecting users that seem much higher priority to me: https://github.com/Homebrew/brew/issues?q=is%3Aissue+is%3Aopen+label%3A%22install+from+api%22 These are blocking our ability to say in #14592 "hey, try enabling API support again". |
I only maintain the main cask repo because the others have increasingly hand-wavy rules so for me keeping them all in the same repo with the same rules would be a big improvement. Also note that Homebrew/homebrew-cask#112102 has existed for 1,5 years at this point. Clearly nobody objects significantly. |
Let's keep that separate for now given we'd ideally look at those in the short term and cask-fonts in the medium term (sharding needs to happen before adding another 2000 casks, and versions & drivers should be tackled first).
Users don't really care as much about taps so maintainability is always going to be an important factor. The user experience can definitely be improved regardless of the approach and I don't agree it's impossible to make a good user experience with separate taps - it's more that we've not committed to an approach yet so there's not been any point putting time in.
Indeed there hasn't been any objections to |
My point was intended to be: I think all of those issues are much more important than fixing search for taps.
Users indirectly care about taps when they don't discover things because they don't know about non-default taps.
I don't think this either. I just think it requires a bunch of work that would be unnecessary for maintainers, contributors and users when you subdivide things into taps. This is why we stopped doing this (very) long ago for formulae. |
For JSON API, these things can even be in the same JSON while technically separate taps under the hood. AFAICT that requires very few changes to do. There's a lot more flexibility now than there was years ago. My point is, I'd like to discuss the driving motivations from a maintainer POV first, and see what the initial preference on that is and whether any workflows can be improved, rather than drive through the migration and considering maintainability later.
I agree with that: though I'd also say they're more important than migrating homebrew-cask-fonts. |
|
Thanks again @Bo98! |
Option A: Algolia
Option B (this PR): remove remote searching entirely
Given the use of our own JSON API, the need for remote searching has largely diminished as you can no longer "untap" Homebrew/cask.
However with both options we do lose the ability to search cask-versions/drivers/fonts when they are untapped, which was the reason the Algolia pull request was rejected so this PR is perhaps not enough of an improvement over the original proposal (though the current functionality is now broken without API tokens). If any ever get added to formulae.brew.sh (even if just as HTML with no JSON), then Algolia could cover them.
Option C is to use the GitHub files API which I could look at, though will require a number of API requests (per repository and perhaps pagination but I haven't checked).
Fixes #15208
Closes #15107