Skip to content

Conversation

irevoire
Copy link
Contributor

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:

  • 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!

@sanders41
Copy link
Contributor

@irevoire just an FYI based on your comment, you can add people as a co-author of the PR if you want.

https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line

@irevoire irevoire force-pushed the fix/remove-document-trait branch from fb2fea0 to 1373183 Compare April 21, 2022 12:32
@irevoire
Copy link
Contributor Author

Oh, thank you! I did that 😁

@irevoire irevoire force-pushed the fix/remove-document-trait branch from 1373183 to d96b44e Compare April 21, 2022 12:35
@irevoire irevoire force-pushed the fix/remove-document-trait branch from 521ca58 to 154e8b8 Compare April 21, 2022 16:43
@brunoocasali brunoocasali added the breaking-change The related changes are breaking for the users label Apr 22, 2022
let client = Client::new("http://localhost:7700", "masterKey");
#[derive(Serialize, Deserialize, Debug)]
#[derive(Serialize, Deserialize )]
Copy link
Member

Choose a reason for hiding this comment

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

Extra space detected 🤖

/// ```
pub async fn get_settings(&self) -> Result<Settings, Error> {
Ok(request::<(), Settings>(
request::<(), Settings>(
Copy link
Member

Choose a reason for hiding this comment

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

Can you give me some idea behind this change? Why it was required before and now isn't?

@brunoocasali brunoocasali self-requested a review April 22, 2022 01:27
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @honne23 and @irevoire you're the best! :)

@brunoocasali
Copy link
Member

bors merge

bors bot added a commit that referenced this pull request Apr 22, 2022
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

<!-- Please link the issue you're trying to fix with this PR, if none then please create an issue first. -->

## 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]>
@bors
Copy link
Contributor

bors bot commented Apr 22, 2022

Build failed:

@brunoocasali
Copy link
Member

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 22, 2022

Build succeeded:

@bors bors bot merged commit ec1addf into main Apr 22, 2022
@bors bors bot deleted the fix/remove-document-trait branch April 22, 2022 01:41
bors bot added a commit that referenced this pull request Apr 25, 2022
272: Update version for the next release (v0.16.0) r=brunoocasali a=brunoocasali

### Release notes:

## ⚠️ Breaking changes

* Use an `OffsetDateTime` on the `ClientStats` (#244) `@irevoire`
* Fixed formatting with clippy & Removed `Document` trait (#267) `@irevoire`

## 🚀 Enhancements

* Add methods to automatically add/update documents in batches (#262) `@abhizer`
* Feature/Tenant Token (#263, #264) `@brunoocasali`

## Misc

* Create an example showing how to update the Settings (#245) `@irevoire`

Thanks again to `@abhizer,` `@bidoubiwa,`  `@brunoocasali,` `@irevoire,`  and Adrian Coutsoftides! 🎉


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

breaking-change The related changes are breaking for the users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove Document trait asking for UIDType

4 participants