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.

Sign up for GitHub

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

Jump to bottom

Implement faceting #39

Merged
merged 13 commits into from
Jun 12, 2020
Merged

Implement faceting #39

merged 13 commits into from
Jun 12, 2020

Conversation

eskombro
Copy link
Member

@eskombro eskombro commented Jun 8, 2020 •
edited
Loading

  • Implement faceting setting routes
  • Implement facetsDistribution search parameter
  • Implement facetFilters search parameter
  • Improve search tests (add test for existing params and add facetsDistribution and facetFilters)
  • add faceting routes tests

For obvious reasons, tests won't pass as faceting is not compatible with the current latest release (v0.10.1)

@eskombro eskombro changed the base branch from master to bump-meilisearch-v0.11 June 8, 2020 23:10
@eskombro eskombro force-pushed the implement_faceting branch from bf72bea to 4358dd1 Compare June 11, 2020 01:15
@eskombro eskombro marked this pull request as ready for review June 11, 2020 01:17
@eskombro eskombro requested review from curquiza and alexisvisco June 11, 2020 01:17
curquiza
curquiza requested changes Jun 11, 2020
View reviewed changes
Copy link
Member

@curquiza curquiza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You did a huge work @eskombro!! Thanks!

My requested changes are about typos that were there before, and a little question from a curious girl 😇

types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Show resolved Hide resolved
eskombro and others added 2 commits June 11, 2020 12:12
@eskombro @curquiza
Fix typos
55d3d71 
Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
@eskombro
More test improvements
45d36c4
@eskombro eskombro force-pushed the implement_faceting branch from aeda165 to 45d36c4 Compare June 11, 2020 10:45
@eskombro eskombro requested a review from curquiza June 11, 2020 10:46
alexisvisco
alexisvisco reviewed Jun 11, 2020
View reviewed changes
client_search.go Outdated Show resolved Hide resolved
alexisvisco
alexisvisco reviewed Jun 11, 2020
View reviewed changes
client_search.go Outdated Show resolved Hide resolved
alexisvisco
alexisvisco reviewed Jun 11, 2020
View reviewed changes
client_search.go Outdated Show resolved Hide resolved
alexisvisco
alexisvisco suggested changes Jun 11, 2020
View reviewed changes
Copy link
Contributor

@alexisvisco alexisvisco left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great job, however there is some mistakes that need to be fix :)

@eskombro eskombro force-pushed the implement_faceting branch from 3b0cbec to 8ba0a6c Compare June 12, 2020 00:32
@eskombro eskombro requested a review from alexisvisco June 12, 2020 00:32
@eskombro
Copy link
Member Author

Thanks for your review @alexisvisco ! I made the changes you proposed!

alexisvisco
alexisvisco reviewed Jun 12, 2020
View reviewed changes
client_documents_test.go Outdated
@@ -9,6 +9,12 @@ type docTest struct {
Name string `json:"name"`
}

type docTestBooks struct {
BookId int `json:"book_id"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhhh So close to the goal 😆
BookdID not BookId (in go an acronym is always in uppercase)

alexisvisco
alexisvisco suggested changes Jun 12, 2020
View reviewed changes
Copy link
Contributor

@alexisvisco alexisvisco left a comment

Choose a reason for hiding this comment

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

So close, it's ok then :)

@eskombro eskombro merged commit ecb785e into bump-meilisearch-v0.11 Jun 12, 2020
@eskombro eskombro deleted the implement_faceting branch June 12, 2020 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexisvisco alexisvisco alexisvisco requested changes

@curquiza curquiza Awaiting requested review from curquiza

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eskombro @alexisvisco @curquiza