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

Fixing piles of clippy errors. #665

Merged
merged 1 commit into from Oct 20, 2022
Merged

Conversation

ehiggs
Copy link
Contributor

@ehiggs ehiggs commented Oct 13, 2022

Related issue

No issue fixed. Simply cleaning up some code for clippy on the march towards a clean build when #659 is merged.

What does this PR do?

Most of these are calling clone when the struct supports Copy.

Many are using & and &mut on self when the function they are called from already has an immutable or mutable borrow so this isn't needed.

I tried to stay away from actual changes or places where I'd have to name fresh variables.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Most of these are calling clone when the struct supports Copy.

Many are using & and &mut on `self` when the function they are called
from already has an immutable or mutable borrow so this isn't needed.

I tried to stay away from actual changes or places where I'd have to
name fresh variables.
@curquiza
Copy link
Member

Hello @ehiggs

thanks for your PR!

bors try

bors bot added a commit that referenced this pull request Oct 17, 2022
@bors
Copy link
Contributor

bors bot commented Oct 17, 2022

@ehiggs
Copy link
Contributor Author

ehiggs commented Oct 19, 2022

Glad to help! @ManyTheFish let me know if there are any changes or explanations needed.

@curquiza
Copy link
Member

curquiza commented Oct 19, 2022

@ehiggs thanks for your PR. The team has a lot to do currently because of our sprint + hacktoberfest. We will do our best to review your PR asap 😄

Thanks again for your work, this is really appreciated

Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

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

Hello @ehiggs, I approve this PR because there is no issue here and your work is really valuable, thanks for this. 👍
However, it changes a significant amount of files and I'm a bit concerned by the merge conflicts that people will have. 🤔

@curquiza
Copy link
Member

@ManyTheFish what is the best solution then? Which PRs can be problematic?

@ehiggs
Copy link
Contributor Author

ehiggs commented Oct 19, 2022

How do I set a label like "no breaking" for the Enforce PR labels?

@ehiggs
Copy link
Contributor Author

ehiggs commented Oct 19, 2022

However, it changes a significant amount of files and I'm a bit concerned by the merge conflicts that people will have. thinking

I understand the concern, which is why I only did some of the fixes. If someone needs help with rebasing, let me know and I can help.

@ManyTheFish ManyTheFish added the no breaking The related changes are not breaking (DB nor API) label Oct 19, 2022
@ManyTheFish
Copy link
Member

I think we can merge this PR,
There is not so much external contribution now, sorry @ehiggs for the time!
Thank you for your contribution! 👍

bors merge

@bors
Copy link
Contributor

bors bot commented Oct 20, 2022

@bors bors bot merged commit f11a408 into meilisearch:main Oct 20, 2022
@meili-bot
Copy link
Contributor

This message is sent automatically

Thank you for contributing to Meilisearch. If you are participating in Hacktoberfest, and you would like to receive some gift from Meilisearch too, please complete this form.

@ehiggs ehiggs deleted the minor-clippy-fixes branch October 20, 2022 11:22
@ehiggs ehiggs mentioned this pull request Oct 24, 2022
3 tasks
bors bot added a commit that referenced this pull request Oct 26, 2022
668: Fix many Clippy errors part 2 r=ManyTheFish a=ehiggs

This brings us a step closer to enforcing clippy on each build.

# Pull Request

## Related issue
This does not fix any issue outright, but it is a second round of fixes for clippy after #665. This should contribute to fixing #659.

## What does this PR do?

Satisfies many issues for clippy. The complaints are mostly:

* Passing reference where a variable is already a reference.
* Using clone where a struct already implements `Copy`
* Using `ok_or_else` when it is a closure that returns a value instead of using the closure to call function (hence we use `ok_or`)
* Unambiguous lifetimes don't need names, so we can just use `'_`
* Using `return` when it is not needed as we are on the last expression of a function.

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [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: Ewan Higgs <ewan.higgs@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants