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(CI): fixing clippy errors on a recent Rust version #567

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

geekbrother
Copy link
Contributor

Description

This PR fixes clippy errors and warnings on the latest Rust version that blocks the CI/CD pipeline.

How Has This Been Tested?

Not tested.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@geekbrother geekbrother self-assigned this Jan 8, 2025
@@ -25,8 +25,6 @@ pub struct LocalConfiguration {
pub postgres_url: String,
#[serde(default = "default_postgres_max_connections")]
pub postgres_max_connections: u32,
#[serde(default = "default_redis_url")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never used warning fix.

@@ -53,10 +51,6 @@ pub fn default_postgres_url() -> String {
"postgres://postgres:postgres@localhost:5432/postgres".to_owned()
}

pub fn default_redis_url() -> String {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never read warning fix.

@@ -644,11 +644,10 @@ impl From<SubscriberWithProjectResult> for SubscriberWithProject {
unread_notification_count: val
.unread_notification_count
.try_into()
.map_err(|e| {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inspect_err must be used warning fix.

@@ -19,13 +19,13 @@ pub enum Addr<'a> {
Separate { read: &'a str, write: &'a str },
}

impl<'a> Default for Addr<'a> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a lifetime a warning fix.

@geekbrother geekbrother marked this pull request as draft January 8, 2025 16:43
// This error shouldn't happen so not bothering with returning Result here
// But if it does, this is a bug and apply use defensive programming
error!("Error converting unread_notification_count from i64 to u64: {e}");
e
error!("Error converting unread_notification_count from i64 to u64");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are no longer logging e, why not?

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.

2 participants