Conversation
osmaczko
force-pushed
the
fix/ci-checks
branch
2 times, most recently
from
9babdf1 to
f20f9f2
Compare
jazzz
left a comment
jazzz
left a comment
There was a problem hiding this comment.
These are changes that have been bugging me. Thanks for taking care of them.
Pre-approving, but I'd like to see a plan for exposing conversation_ids() over FFI to make sure that work doesn't get lost.
|
|
||
| impl Id for GroupTestConvo { | ||
| fn id(&self) -> ConversationId { | ||
| fn id(&self) -> ConversationId<'_> { |
There was a problem hiding this comment.
This elided lifetime here is not necessary.
Is this a specific style choice?
There was a problem hiding this comment.
Not a style choice. The <'_> is actually required by the mismatched_lifetime_syntaxes lint (which is promoted to an error by -D warnings in CI). Without it, clippy fails.
Since ConversationId<'a> is a type alias for &'a str, using it without <'_> in fn id(&self) -> ConversationId hides the lifetime relationship with &self.
| pub fn conversation_ids(&self) -> Vec<ConversationIdOwned> { | ||
| self.conversations.keys().cloned().collect() | ||
| } |
There was a problem hiding this comment.
This function needs to be exposed via FFI. Rather than deleting it I would suggest creating a ticket to expose it - and resolve the warning that way.
There was a problem hiding this comment.
Good call, added it back with #[allow(dead_code)] until it gets exposed via FFI.
osmaczko
force-pushed
the
fix/ci-checks
branch
from
f20f9f2 to
0082967
Compare
2 participants
No description provided.