[tech] Update to clippy 1.50.0 #744
Conversation
model-builder/src/builder.rs
Outdated
| @@ -239,25 +239,25 @@ impl IntoTime for &str { | |||
| } | |||
| } | |||
|
|
|||
| pub trait IntoDate { | |||
| fn into_date(&self) -> Date; | |||
| pub trait IntoDate: Copy { | |||
There was a problem hiding this comment.
A clippy lint was complaining about naming a function into_*() while not taking self. At first, I thought we might make an exception but at the same time, the trait IntoTime takes self but IntoDate takes &self so I found that was inconsistent.
However, since fn calendar() takes a &[impl IntoDate], it was impossible to get ownership in this case... unless making IntoDate also Copy. That what I did but I'm open for discussion and alternative. At the very least, I don't want to hide this change 😄
cc @antoine-de
There was a problem hiding this comment.
Hum I don't know, it might be a hard requirement on the Trait 🤔
maybe we can just rename the Trait to AsDate (and have a as_date(&self) method)?
There was a problem hiding this comment.
I thought about that too but .as_date() is not really correct either for the &str implementation since a new object is created in this case.
I thought about Copy since I didn't expect any other implementation than Date, &Date and &str which are all Copy.
In the end, I'm fine with either solution.
There was a problem hiding this comment.
since Date is Copy I don't think it's important if a as_ returns a Date and not a &Date is it?
There was a problem hiding this comment.
I've changed it for AsDate.
2d2d251 to
2e24f7c
Compare
No description provided.