-
Notifications
You must be signed in to change notification settings - Fork 5
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
Bumped down limits on gifs images and youtube #44
Conversation
That should hopefully make the results a little more accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
lisa_commands/request_helper.py
Outdated
@@ -147,7 +147,7 @@ def youtube_search(query): | |||
'part': 'snippet', | |||
'q': query, | |||
'key': GOOGLE_API_KEY, | |||
'maxResults': 10 | |||
'maxResults': 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we don't want to put youtube to first result only? In almost all youtube me
cases I can think of, I feel like people generally want to find a specific video. It's not quite like image me
where there can be multiple acceptable images, so it just needs to grab any one of them.
Edit: I guess for songs or something you can possibly get multiple good results near the top of the list, but even at 3 there's a good chance of grabbing live versions when you wanted the studio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a super strong objection to that. If no one else weighs in then I'll do that.
lisa_commands/request_helper.py
Outdated
@@ -192,7 +192,7 @@ def gify_search(media_type, query): | |||
:query what to query for | |||
""" | |||
|
|||
limit = 10 | |||
limit = 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to have gif limit different than other image searching? I imagine the use cases are almost identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seconding this, gif searches can probably be a bit more lax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, comment is a minor nit that can be ignored if desired
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment can also be ignored at the bot-maker's discretion if desired
4ceb155
I went ahead and did the merge cause it's was literally changing some numbers and one line of code after review |
Hopefully help address #40