Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Introduce the reset_sortable_fields Settings method #330

Merged
merged 3 commits into from
Aug 30, 2021
Merged

Introduce the reset_sortable_fields Settings method #330

merged 3 commits into from
Aug 30, 2021

Conversation

Kerollmops
Copy link
Member

@Kerollmops Kerollmops commented Aug 25, 2021 •
edited
Loading

I forgot to add the reset_sortable_fields method on the Settings builder, it is no big deal as the library user (like MeiliSearch) can always call set_sortable_fields with an empty list of fields, it is equivalent.

@Kerollmops Kerollmops mentioned this pull request Aug 25, 2021
Merged
@Kerollmops Kerollmops marked this pull request as ready for review August 25, 2021 15:59
@Kerollmops Kerollmops requested a review from ManyTheFish August 25, 2021 15:59
irevoire
irevoire previously approved these changes Aug 26, 2021
View reviewed changes
@curquiza
Copy link
Member

curquiza commented Aug 26, 2021 •
edited
Loading

What does this PR imply to MeiliSearch? Do we need to integrate the usage of reset_sortable_fields? Is set_sortable_fields used in this PR (meilisearch/meilisearch#1615) to reset at the moment? 🙂

@Kerollmops
Copy link
Member Author

Kerollmops commented Aug 26, 2021 •
edited
Loading

What does this PR imply to MeiliSearch? Do we need to integrate the usage of reset_sortable_fields? Is set_sortable_fields used in this PR (meilisearch/MeiliSearch#1615) to reset at the moment? 🙂

Currently, yes, we use the set_sortable_fields in this PR and it is no big deal as using reset is the same as setting an empty HashMap. I just created this PR on milli to make the settings more consistent and we will patch MeiliSearch when we bump the milli's version. No need to create a release just for that 😄

@curquiza curquiza mentioned this pull request Aug 26, 2021
Closed
@Kerollmops Kerollmops mentioned this pull request Aug 30, 2021
Merged
@shekhirin shekhirin mentioned this pull request Aug 30, 2021
Merged
@Kerollmops
Copy link
Member Author

bors merge

bors bot added a commit that referenced this pull request Aug 30, 2021
@bors @Kerollmops
Merge #330
f3c0bef 
330: Introduce the reset_sortable_fields Settings method r=Kerollmops a=Kerollmops

I forgot to add the `reset_sortable_fields` method on the `Settings` builder, it is no big deal as the library user (like MeiliSearch) can always call `set_sortable_fields` with an empty list of fields, it is equivalent.

Co-authored-by: Kerollmops <clement@meilisearch.com>
@Kerollmops
Copy link
Member Author

bors cancel

@bors
Copy link
Contributor

bors bot commented Aug 30, 2021

Canceled.

@bors @irevoire
Merge #332
6cdb672 
332: Sortable attributes in http-ui r=Kerollmops a=irevoire

- Add a `reset_sortable_attribute` method
- Add the `sortable_attributes` to http-ui
- Fix some broken test in http-ui

Co-authored-by: Tamo <tamo@meilisearch.com>
irevoire
irevoire approved these changes Aug 30, 2021
View reviewed changes
Copy link
Member

@irevoire irevoire left a comment

Choose a reason for hiding this comment

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

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 30, 2021

Build succeeded:

@bors bors bot merged commit df38794 into main Aug 30, 2021
@bors bors bot deleted the reset-sortable-attributes branch August 30, 2021 23:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@irevoire irevoire irevoire approved these changes

@ManyTheFish ManyTheFish Awaiting requested review from ManyTheFish

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
@Kerollmops @curquiza @irevoire