Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy warnings #178

Merged
merged 12 commits into from
Jul 11, 2023
Merged

Fix clippy warnings #178

merged 12 commits into from
Jul 11, 2023

Conversation

AureliaDolo
Copy link
Contributor

@AureliaDolo AureliaDolo commented Jul 5, 2023

Hello!

This PR fixes most clippy lints.

For the Safety doc of EnvOpenOptions::flag I retrieved your comment here

The yet to fix lints are :

  • Safety doc for RwRevIter::append
  • Safety doc for Env::copy_to_fd
  • Module inception for iter modules

I'd happily take your input regarding this issues.

Thanks !

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.

Hey @AureliaDolo 👋,

Thank you very much for your PR! It's pretty cool of you.

Could you please rebase on main, as I removed Bors and fixed the CI? After I review this first step of your PR, could you please add a clippy job in the CI?

Ho 😮 I just noticed that the env::EnvInfo struct is not reexported to the lib.rs file, could you fix it here, please? I don't know why the compiler/doc compiler said nothing.

About the safety messages:

Safety doc for RwRevIter::append

That's a really important one, good catch! This message must be copy/pasted for every append method of the crate, but you must not give anything that comes from the database itself. Always make sure that you clone that on the heap before giving it to this function if it's the case!

Safety doc for Env::copy_to_fd

I would say that the main reason is that it exposes a mdb_filehandle_t with either a c_int on UNIX or a void* on Windows. Maybe just a note to that that you must make sure the handle is valid before doing anything. WDTY?

Module inception for iter modules

There are two ways to fix that:

  • Ignore it with an #[allow(clippy::module_inception)].
  • Rename the outer iter module into iterator.

heed/src/env.rs Outdated Show resolved Hide resolved
@AureliaDolo
Copy link
Contributor Author

Thanks for the review !

The renaming of the iterator module should not have an impact on users as iterators are re exported.

Ho 😮 I just noticed that the env::EnvInfo struct is not reexported to the lib.rs file, could you fix it here, please? I don't know why the compiler/doc compiler said nothing.

I think it's because you may have wanted to use EnvInfo in other modules, but not expose it to the great outside

@Kerollmops
Copy link
Member

Looks perfect to me. Thank you very much! Nice job!

@Kerollmops Kerollmops merged commit 2a8ae11 into meilisearch:main Jul 11, 2023
8 checks passed
@Kerollmops Kerollmops added this to the v0.20.0 milestone Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants