Fixed formatting with clippy and removed Document trait #261

honne23 · 2022-04-14T17:43:06Z

Pull Request

What does this PR do?

Fixes #255

PR checklist

Please check if your PR fulfills the following requirements:

Does this PR fix an existing issue?
Have you read the contributing guidelines?
Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

bidoubiwa · 2022-04-14T18:29:30Z

Thanks 🙏

Could you update the .code-samples.meilisearch.yaml file ?

irevoire · 2022-04-14T19:55:44Z

Hey, thanks for your contribution!
I'll review the PR tomorrow or on Monday but it look pretty good 🎉

honne23 · 2022-04-14T19:58:25Z

Thank you everyone, unfortunately my macbook literally just died (screen fried) the moment I pushed up this PR. I'll see how fast I can get a new one but it might delay any further updates from me :/

honne23 · 2022-04-15T15:35:38Z

@bidoubiwa @irevoire back with a new M1 mac and updated the code samples

src/indexes.rs

src/lib.rs

remove unnecessary lifetime syntax Co-authored-by: Tamo <[email protected]>

fix and update import syntax for documentation Co-authored-by: Tamo <[email protected]>

remove unnecessary lifetime syntax from function signature Co-authored-by: Tamo <[email protected]>

fix imports in documentation Co-authored-by: Tamo <[email protected]>

fix parameter value in documentation Co-authored-by: Tamo <[email protected]>

fix imports in documentation Co-authored-by: Tamo <[email protected]>

fix spacing in documentation Co-authored-by: Tamo <[email protected]>

brunoocasali · 2022-04-18T16:48:04Z

@irevoire the changes are breaking somehow for the users?

brunoocasali

@honne23 thanks for your contribution! ❤️

But I need your help to fix the README :)

Check it out: https://github.com/meilisearch/meilisearch-rust/runs/6065376469?check_suite_focus=true

irevoire · 2022-04-18T17:06:12Z

@brunoocasali yes it's breaking

kvinwang · 2022-04-19T00:55:20Z

src/indexes.rs


    /// Alias for [Index::add_or_replace].
-    pub async fn add_documents<T: Document>(
+    pub async fn add_documents<T: DeserializeOwned + Serialize + std::fmt::Debug>(


The DeserializeOwned is not necessary here. Remove it so that we can add docs like this:

#[derive(Serialize, Debug)] struct Movie<'a> { name: &'a str, description: &'a str, }

Hey @kvinwang, just so you have the whole context over this PR.
There was a little discussion on our slack community a few days ago where @honne23 explained to us why the Owned version was needed;

There was a little discussion on our slack community a few days ago where @honne23 explained to us why the Owned version was needed;

The issue here is not about Owned or not. The add_documents only need to serialize the data, So neither DeserializeOwned or Deserialize is needed here.

Ah yes you're right my bad!

kvinwang · 2022-04-19T00:59:46Z

src/indexes.rs

    /// ```
-    pub async fn get_document<T: 'static + Document>(&self, uid: T::UIDType) -> Result<T, Error> {
-        Ok(request::<(), T>(
+    pub async fn get_document<T: 'static + DeserializeOwned+ Serialize>(&self, uid: &str) -> Result<T, Error> {


We can remove the Serialize bound.

kvinwang · 2022-04-19T01:01:05Z

src/indexes.rs

    /// # });
    /// ```
-    pub async fn get_documents<T: 'static + Document>(
+    pub async fn get_documents<T: 'static + DeserializeOwned + Serialize + std::fmt::Debug>(


We can also remove the std::fmt::Debug here from what I see 👍

honne23 · 2022-04-19T10:24:57Z

working on this today, will push this through later

irevoire · 2022-04-19T11:18:21Z

src/indexes.rs

    /// # });
    /// ```
-    pub async fn add_or_replace<T: Document>(
+    pub async fn add_or_replace<T: DeserializeOwned + Serialize + std::fmt::Debug>(


Same here, if we can we should remove the Debug

irevoire · 2022-04-20T08:38:24Z

Hey, @honne23, I would like to merge this PR soon. Do you think you'll be able to work on it today?
Or can I take your PR and implements the last few comments?

irevoire · 2022-04-21T12:21:30Z

Closing in favor of #267

Thanks for your contribution 💝

267: fixed formatting with clippy and removed Document trait r=brunoocasali a=irevoire # Pull Request Since `@honne23` doesn't seem available currently, and we have #262, #263, and #264 waiting for this PR to be merged, I took back on it. But thanks a lot, `@honne23;` you wrote most of the code, and I just applied the few left comments. Thanks for contributing! ## What does this PR do? Fixes #255 Closes #261  ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: Adrian Coutsoftides <[email protected]> Co-authored-by: Bruno Casali <[email protected]>

fixed formatting with clippy and removed Document trait

e7d974d

updated .code-samples.meilisearch.yaml

35c1a46

irevoire reviewed Apr 17, 2022

View reviewed changes

irevoire mentioned this pull request Apr 17, 2022

Add methods to automatically add/update documents in batches #262

Merged

3 tasks

honne23 and others added 9 commits April 17, 2022 16:58

Update src/indexes.rs

1780241

remove unnecessary lifetime syntax Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

d99acd6

fix and update import syntax for documentation Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

ce08108

remove unnecessary lifetime syntax from function signature Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

0afe8c0

fix imports in documentation Co-authored-by: Tamo <[email protected]>

Update src/lib.rs

0ca62d9

fix imports in documentation Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

38178f8

fix parameter value in documentation Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

c12f2c6

fix imports in documentation Co-authored-by: Tamo <[email protected]>

Update src/lib.rs

5fd185b

fix imports in documentation Co-authored-by: Tamo <[email protected]>

Update src/indexes.rs

4db60cd

fix spacing in documentation Co-authored-by: Tamo <[email protected]>

brunoocasali added the enhancement New feature or request label Apr 18, 2022

brunoocasali requested changes Apr 18, 2022

View reviewed changes

brunoocasali mentioned this pull request Apr 18, 2022

Feature/Tenant Token #263

Merged

brunoocasali added the breaking-change The related changes are breaking for the users label Apr 18, 2022

kvinwang reviewed Apr 19, 2022

View reviewed changes

irevoire reviewed Apr 19, 2022

View reviewed changes

irevoire mentioned this pull request Apr 21, 2022

fixed formatting with clippy and removed Document trait #267

Merged

3 tasks

irevoire closed this Apr 21, 2022

brunoocasali added the stale Pull request or issue that has recieve no activity for a long time. label Apr 22, 2022

Fixed formatting with clippy and removed Document trait #261

Fixed formatting with clippy and removed Document trait #261

Uh oh!

Conversation

honne23 commented Apr 14, 2022

Pull Request

What does this PR do?

PR checklist

Uh oh!

bidoubiwa commented Apr 14, 2022

Uh oh!

irevoire commented Apr 14, 2022

Uh oh!

honne23 commented Apr 14, 2022

Uh oh!

honne23 commented Apr 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunoocasali commented Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoocasali left a comment

Choose a reason for hiding this comment

Uh oh!

irevoire commented Apr 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honne23 commented Apr 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irevoire commented Apr 20, 2022

Uh oh!

irevoire commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

brunoocasali commented Apr 18, 2022 •

edited

Loading

irevoire commented Apr 21, 2022 •

edited

Loading