Skip to content
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

feat: image search #204

Closed
wants to merge 49 commits into from
Closed

feat: image search #204

wants to merge 49 commits into from

Conversation

aleksasiriski
Copy link
Member

@aleksasiriski aleksasiriski commented Feb 8, 2024

No description provided.

@aleksasiriski aleksasiriski changed the base branch from main to refactor-search February 8, 2024 13:49
@aleksasiriski aleksasiriski requested a review from k4lizen February 8, 2024 13:50
@aleksasiriski aleksasiriski mentioned this pull request Feb 9, 2024
8 tasks
Base automatically changed from refactor-search to main February 9, 2024 22:52
@aleksasiriski aleksasiriski changed the title feat: Image search feat: image search Feb 13, 2024
@aleksasiriski aleksasiriski changed the title feat: Image search feat: image search Feb 23, 2024
Copy link
Member

@k4lizen k4lizen left a comment

Choose a reason for hiding this comment

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

Since the page url is the one saved in the URL field of Result, this PR currently does not support having two different images come from the same page (since the URL is used as the primary key in the map which collects results). This is a bug. A possible fix is to save the image source url to the URL field, and have an additional field in ImageResult like OriginatingPageURL.

k4lizen
k4lizen previously approved these changes Feb 25, 2024
@aleksasiriski aleksasiriski requested a review from k4lizen March 1, 2024 14:50
@aleksasiriski aleksasiriski changed the title feat: image search feat(search): images Mar 1, 2024
@aleksasiriski aleksasiriski changed the title feat(search): images feat: image search Mar 1, 2024
@aleksasiriski aleksasiriski mentioned this pull request Mar 1, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants