@generall

After PR#52 There are 2 types of tricky ones left:

warning: writing `&String` instead of `&str` involves a new object where a slice will do
   --> lib/collection/src/segment_manager/simple_segment_updater.rs:205:21
    |
205 |         field_name: &PayloadKeyType,
    |                     ^^^^^^^^^^^^^^^ help: change this to: `&str`
    |
    = note: `#[warn(clippy::ptr_arg)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

This is pretty much hitting the case discussed here. There is a lot of occureneces.
There are 4 ways to address it:

Disable the rule globally, but that would cause a potential misses of good pattens
Define PayloadKeyTypeRef as &str alias and use it instead of &PayloadKeyType
Do not use custom type PayloadKeyType alias, use String and &str (suggested in here)
Disable the rule locally by putting #[allow(clippy::ptr_arg)] on top of every affected method

I personally would suggest going with option 3 or 4. What do you think?

The second one is easier:

warning: module has the same name as its containing module
 --> lib/collection/src/collection_builder/mod.rs:1:1
  |
1 | pub mod collection_builder;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(clippy::module_inception)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_inception

There are multiple cases like that, the above is just an example.
The leads to rather confusing use statements like:

use crate::collection_builder::collection_builder::construct_collection;

where collection_builder is written twice
There are 4 solutions here again

Disable it
Rename internal module
Move the implementation of the collection_builder into mod.rs level (this is not actually considered as a bad practice IMHO)
Rename the internal module, remove pub in mod.rs against it and reexport using pub use collection_builder::*. The above use statement would become use crate::collection_builder::construct_collection; then

I personally just would vote for option 4 here or otherwise for option 2. Again, let me know what do you think.

Once those 2 are fixed, we could add clippy check to CI pipelines

Clean the project using clippy #45

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions