Skip to content

Conversation

irevoire
Copy link
Contributor

This PR unifies the two types Settings.
It also gets rids of the IntoVecString in favor of a simpler IntoIterator<Item = AsRef<str>>, which is more versatile and common in rust.

get rid of the IntoVecString trait

Apply review comments

Co-authored-by: Clément Renault <[email protected]>
Copy link
Member

@Kerollmops Kerollmops left a 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! Thank you, Tamo!

@curquiza
Copy link
Member

Is it breaking for the user or is it only internal refactoring?

@irevoire
Copy link
Contributor Author

It's only internal and should be more flexible for the user.
Instead of accepting a subset of type when updating a settings now we accept every type that where handled + a lot of new types

@curquiza curquiza added the enhancement New feature or request label Feb 10, 2022
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

💪

@curquiza
Copy link
Member

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 10, 2022

Build succeeded:

@bors bors bot merged commit 2d12b3b into meilisearch:main Feb 10, 2022
bors bot added a commit that referenced this pull request Feb 16, 2022
243: Update version for the next release (v0.15.0) r=brunoocasali a=brunoocasali

## ⚠️ Breaking changes

* Refactorise the errors, now `error_code` and `error_type` has types instead of `string` (#234) `@irevoire`
* Put type on timestamp and duration (#237) `@irevoire`
  * The `time` crate will be needed if you need to manipulate API keys (check for more info #226, #237 (comment), and meilisearch/integration-guides#121 (comment)).

## 🚀 Enhancements

* Refactorise the settings (#235) `@irevoire`

Thanks again to `@irevoire!` 🎉


Co-authored-by: Bruno Casali <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants