-
Notifications
You must be signed in to change notification settings - Fork 138
refactor: enable missing_panics_doc
clippy lint
#1006
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
base: master
Are you sure you want to change the base?
Conversation
RRRadicalEdward
commented
Oct 3, 2025
> Checks the doc comments of publicly visible functions that may panic and warns if there is no # Panics section.
Coverage Report 🤖 ⚙️Past: New: Diff: -0.00% [this comment will be updated automatically] |
Ok(u64::from_le_bytes(self.data.as_ref().try_into().map_err(|_| { | ||
invalid_field_err!( | ||
"requestedFileContentsData", | ||
"Invalid data size for u64 size" | ||
)); | ||
} | ||
|
||
Ok(u64::from_le_bytes( | ||
self.data | ||
.as_ref() | ||
.try_into() | ||
.expect("data contains exactly eight u8 elements"), | ||
)) | ||
"Can't cast self.data to u64 via &[u8, 8], invalid data size" | ||
) | ||
})?)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I think this is the most readable as of today:
let chunk = self.data.first_chunk::<8>().ok_or_else(|| invalid_field_err!(
"requestedFileContentsData",
"not enough bytes for u64 size"
))?;
Ok(u64::from_le_bytes(chunk))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to use try_into
. The way it was, but without the expect
.
/// # Panics | ||
/// | ||
/// If log directory creation fails. | ||
/// | ||
/// If tracing initialization fails. | ||
pub fn init_with_env() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: It’s actually not okay to panic here. Can you add a FIXME?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Does it look good?